.github: Add issue, PR templates #873
Conversation
ix5
added
the
meta
| creating changes. | ||
| The following items are listed to help you create a great Pull Request: --> | ||
| ## Checklist | ||
| - [ ] All new and existing **tests are passing** |
There was a problem hiding this comment.
Why does this need a checklist - doesn't running the github actions cover this?
There was a problem hiding this comment.
- Ideally, GH Actions should not be the first instance PR authors come across the tests. Getting the tests to pass by pushing to (re-)trigger CI runs would be very tedious - we'd want authors to have the tests running locally so they can understand and fix errors faster.
I'd also wager a lot of authors most likely won't even register the tiny red/green check marks. - Spelling the need for working tests out explicitly (even if it may be a repetition of the "contributing" section of the docs) is a good reminder for authors, even already when thinking about submitting drafts
There was a problem hiding this comment.
Using some sort of CI system (all of which are reported with check masks) is common for everything on GitHub. The more text you include here, the less likely people are to actually read all of it.
There was a problem hiding this comment.
I've reduced the amount of text and bullet points even further.
I am convinced and have seen evidence of this type of checklist being useful - and though I hate this type of argument, it's also something other successful projects do.
|
As mentioned earlier, I'm not hard opposed but also not convinced templates solve concrete problems we have. |
ix5
force-pushed
the
gh-pull-request-templates
branch
2 times, most recently
from
e59409b to
075f11f
Compare
|
I'm still waiting for an answer from posativ regarding new uploads of the docs so I can link the contributing and testing guides. |
ix5
force-pushed
the
gh-pull-request-templates
branch
from
075f11f to
b03caa5
Compare
ix5
force-pushed
the
gh-pull-request-templates
branch
from
b03caa5 to
3642721
Compare
| - [ ] (If adding features:) I have added tests to cover my changes | ||
| - [ ] (If docs changes needed:) I have updated the **documentation** accordingly. | ||
| - [ ] I have added an entry to `CHANGES.rst` because this is a user-facing change or an important bugfix | ||
| - [ ] I have written **proper commit message(s)** |
There was a problem hiding this comment.
This is the wrong place for this, it belongs in a CONTRIBUTING.md or something similar; by the time the contributor creates the pull request they've already written these commit messages so this is the wrong moment to find out about the right format.
There are also loads of other things we could be checking here; did they pick reasonable variable names for example?
There was a problem hiding this comment.
All perfectly valid points. Since docs on posativ.org are outdated for now, maybe we could link to the source doc as rendered here: isso@master:docs/contributing/index.rst
|
I'm still not convinced this helps, but also don't want to stand in the way so I'll abstain. |
|
We'll revisit once updated docs are uploaded in the proper place and we can have a |
Encourage users to fill out some basic information and guide their thought processes.
As opposed to #753, these templates are much less intrusive/inquisitive and merely guide the user instead of demanding excessive amounts of information.
Reasoning: While reviewing PRs of more or less new contributors to the project such as @BBaoVanC and @fliiiix, I had to coax their PRs metadata into a more reviewable shape. BBaoVanC even ended up writing checklists for themself. Formalizing this process a bit makes sense, IMO.
@jelmer thoughts?