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

Adding more items in PR checklists. #24245

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,8 @@ We use the [Fork and Pull model](https://docs.github.com/en/pull-requests/collab
- Make sure you follow the [review and commit guidelines](https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines), in particular:

- Ensure that each commit is correct independently. Each commit should compile and pass tests.
- When possible, reduce the size of the commit for ease of review.
- Squash all merge commits before the PR is rebased and merged.
- When possible, reduce the size of the commit for ease of review. Consider breaking a large PR into multiple commits, with each one addressing a particular issue. For example, if you are introducing a new feature that requires certain refactor, making a separate refactor commit before the real change helps the reviewer to isolate the changes.
- Do not send commits like addressing comments or fixing tests for previous commits in the same PR. Squash such commits to its corresponding base commit before the PR is rebased and merged.
- Make sure commit messages [follow these guidelines](https://chris.beams.io/posts/git-commit/). In particular (from the guidelines):

* Separate subject from body with a blank line
Expand All @@ -463,13 +463,14 @@ We use the [Fork and Pull model](https://docs.github.com/en/pull-requests/collab
* Use the imperative mood in the subject line
* Wrap the body at 72 characters
* Use the body to explain what and why vs. how
* Ensure all code is peer reviewed within your own organization or peers before submitting
* Implement and address existing feedback before requesting further review
* Make a good faith effort to locate example or referential code before requesting someone else direct you towards it
* If you see a lack of documentation, create a separate commit with draft documentation to fill the gap
- Ensure all code is peer reviewed within your own organization or peers before submitting
- Implement and address existing feedback before requesting further review
- Make a good faith effort to locate example or referential code before requesting someone else direct you towards it
- If you see a lack of documentation, create a separate commit with draft documentation to fill the gap
* This documentation can be iterated on same as any code commit, demonstrate in real time that you are learning the code section
* Implement or modify relevant tests, otherwise provide clear explanation why test updates were not necessary
* Tag your PR with affected code areas as best as you can, it’s okay to tag too many, better to cut down irrelevant tags than miss getting input from relevant subject matter experts
- Implement or modify relevant tests, otherwise provide clear explanation why test updates were not necessary
- Tag your PR with affected code areas as best as you can, it’s okay to tag too many, better to cut down irrelevant tags than miss getting input from relevant subject matter experts
- All tests shall pass before requesting a code review. If there are test failures, even it's from unrelated problems, try to address them by either sending a PR to fix it or creating a Github issue so it can be triaged and fixed soon.

### What not to do for Pull Requests
* Submit before getting peer review in your own organization
Expand All @@ -478,6 +479,7 @@ We use the [Fork and Pull model](https://docs.github.com/en/pull-requests/collab
* Protest lack of documentation for a code section
* Instead, review the related code, then draft initial documentation as a separate commit
* Submit without test cases or clear justification for lack thereof
* Request review when there are tests failing

### Comments in Pull Requests
Comments should help move the PR toward completion.
Expand Down
Loading