Conversation
The current template is quite wordy and hard to read. We have a properly rendered checklist in `CONTRIBUTING.md`, so it's clearest just to link to that; I'm not convinced that the existing comment actually drives user behaviour to do what we want, so I don't think we'll lose out by putting it a link away. I deleted the "summary" and "details and comments" section headings because they're usually below the fold of the text area anyway, and just busy-work to delete for PRs that come with a good commit message. One thing we definitely _do_ want in all PR descriptions, and is not possible for maintainers to manually do themselves, is to include a disclosure of the AI/LLM tooling used. This new template pre-populates that with tickboxes, including for the negative case, which should at least make it obvious to reviewers if all the information is present.
jakelishman
added
the
Changelog: None
|
One or more of the following people are relevant to this code:
|
|
+1 on removing the wordy comment header, but I'm actually using the summary and details bits more often than not -- but I'm obeying the majority opinion here 🙂 |
|
I didn't mean to imply that you shouldn't put in a summary/detail, just that they don't need to go under a specific heading - the text just needs to exist in any form. |
| - [ ] I have added the tests to cover my changes. | ||
| - [ ] I have updated the documentation accordingly. | ||
| - [ ] I have read the CONTRIBUTING document. |
There was a problem hiding this comment.
Should we really get rid of this part? I mean. I know some contributions don't require us to write tests, or update docs, but it is a good reminder to have.
There was a problem hiding this comment.
I would suggest to keep them and also mention release notes:
I have updated the documentation and release notes accordingly.
There was a problem hiding this comment.
and perhaps also add another item on format checking?
(it seems that most comments that I write deal with failing tests / missing release notes / format errors...) - maybe we should use LLMs to automate this?
There was a problem hiding this comment.
The question is how detailed we want to be here: the checklist for a PR is already in the linked contributing guide, so we don't have to duplicate it here. I think I'd be in favor of trying a minimal template and explicitly point out the guide. If we realize it doesn't work and PR description drops, we can still change it
There was a problem hiding this comment.
I agree, but I'm afraid that human users don't actually read the guidelines (and I'm not sure about LLMs ;)
Perhaps we can use some automated Bot that will comment on missing CLA / tests / release notes / formatting issues...
There was a problem hiding this comment.
Not every PR needs tests or release notes -- and we do have a CLA bot and formatting issues are caught by CI 🙂
There was a problem hiding this comment.
I think I'd be in favor of trying a minimal template and explicitly point out the guide. If we realize it doesn't work and PR description drops, we can still change it
Shall we try this for now? I'm happy to do this and then change again if we don't like it.
raynelfss
left a comment
raynelfss
left a comment
There was a problem hiding this comment.
Based on the discussion above about my comments. I don't feel strongly about anything else in this template.
5 participants
The current template is quite wordy and hard to read. We have a properly rendered checklist in
CONTRIBUTING.md, so it's clearest just to link to that; I'm not convinced that the existing comment actually drives user behaviour to do what we want, so I don't think we'll lose out by putting it a link away.I deleted the "summary" and "details and comments" section headings because they're usually below the fold of the text area anyway, and just busy-work to delete for PRs that come with a good commit message.
One thing we definitely do want in all PR descriptions, and is not possible for maintainers to manually do themselves, is to include a disclosure of the AI/LLM tooling used. This new template pre-populates that with tickboxes, including for the negative case, which should at least make it obvious to reviewers if all the information is present.
Summary
Details and comments