Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp: relaxing pull request template #51

Open
JaeAeich opened this issue Jan 7, 2025 · 2 comments · May be fixed by #58
Open

imp: relaxing pull request template #51

JaeAeich opened this issue Jan 7, 2025 · 2 comments · May be fixed by #58

Comments

@JaeAeich
Copy link
Collaborator

JaeAeich commented Jan 7, 2025

Pull request template has a long list of checklist, I propose to get rid of them because:

  • Some of which are unnecessary because we have string CI checks for those.
  • Fr the rest of them, going though all the checks is a waste of time. I would rather have a checklist saying something like "By creating this PR I have agree that I have not reduced the test coverage."
  • Lastly and probably most importantly, GH considers these checklists as task, so for each PR even though we have only solved one issues its shows 12 task completed which is misleading.
@JaeAeich
Copy link
Collaborator Author

JaeAeich commented Jan 7, 2025

@uniqueg thoughts?

@uniqueg
Copy link
Member

uniqueg commented Jan 8, 2025

  1. By all means, remove everything that we have reliable automated checks for.
  2. I disagree. Or rather, I only agree that it's a waste of time if people just tick the boxes unthinkingly. And I think we can reduce the risk of contributors doing that by concise language (maybe with links to the contributor guide for more details) and listing only those items that are not automatically checked. If during review, we then still find that contributors are ticking boxes heedlessly, we should hold them accountable. If contributors are ticking boxes truthfully, an overhead of a few minutes to take a load of maintainers and reviewers is absolutely acceptable and part of a developer's job. Neither me nor anyone else should have to deal with low quality issues, PRs etc. So rather than removing the checklist, I would like to add one item that says sth to the effect of "I acknowledge that my PR will not be reviewed until all items in this checklist are ticked and all automated checks pass successfully". Of course we need to phrase the checklist items in a way that they can be truthfully checked if they don't apply to a particular PR - but I think it's fine to push contributors to at least think about whether an item applies or not.
  3. I don't see a problem with that, they are tasks. So on the contrary, I think it's quite useful, because, as a reviewer, it allows me to easily spot PRs that are not ready (because not ticking all the boxes means that a PR is not ready) and ignore them (as an additional hint to the red icon for failed checks). If I want to know how many issues are linked, I can easily see that in the appropriate column in the PRs overview.

@JaeAeich JaeAeich linked a pull request Jan 9, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants