refactor: Update PR template. #533
Conversation
| @@ -6,12 +6,11 @@ Please provide a paragraph or two giving a summary of the change, including rele | |||
|
|
|||
| - [ ] I have reviewed my diff in github, line by line. | |||
| - [ ] Every change is related to the PR description. | |||
| - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to the issue(s) that it resolves. | |||
| - [ ] The branch has been merged with/rebased against the head of its merge target. | |||
There was a problem hiding this comment.
This is more important, hence higher up the list. Also people can use merges as long as they know the difference vs a rebase.
| @@ -6,12 +6,11 @@ Please provide a paragraph or two giving a summary of the change, including rele | |||
|
|
|||
| - [ ] I have reviewed my diff in github, line by line. | |||
| - [ ] Every change is related to the PR description. | |||
| - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to the issue(s) that it resolves. | |||
There was a problem hiding this comment.
This is less important and is rephrased to not imply that every PR must correspond to an issue.
| - [ ] There are no unexpected formatting changes, superfluous debug logs, or commented-out code. | ||
| - [ ] There are no circuit changes, OR specifications in `/markdown/specs` have been updated. |
There was a problem hiding this comment.
This directory doesn't exist anymore.
| - [ ] There are no circuit changes, OR a cryptographer has been assigned for review. | ||
| - [ ] I've updated any terraform that needs updating (e.g. environment variables) for deployment. |
There was a problem hiding this comment.
There is no Terraform anywhere in the repo. This is a holdover from the monorepo days.
| - [ ] New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have `@brief` describing the intended functionality. | ||
| - [ ] If existing code has been modified, such documentation has been added or updated. | ||
| - [ ] No superfluous `include` directives have been added. |
There was a problem hiding this comment.
There has been an explosion of includes, in large part due to some badly chosen clangd settings. That should be fixed now, and we should make sure we're not lazily adding includes that don't need to be there.
There was a problem hiding this comment.
"I have removed one superfluous include in the interest of the common good"
|
All looks good to me. I can't think of anything else I would add. I'd like to generally enforce the use of smaller more well-scoped PRs (insofar as is possible) but perhaps the time to do that is at the conception of the work, not this final stage. |
Description
PR template needs sprucing up.
Checklist:
Just look at this thing
/markdown/specshave been updated.@briefdescribing the intended functionality.