From 3ada782e8d0007f692eb6c08c481887a77ff8461 Mon Sep 17 00:00:00 2001 From: Ying <33299678+yingsu00@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:57:25 -0800 Subject: [PATCH] Adding more items in PR checklists. (#24245) Adding more items in PR checklists. Correct the bullet point indentations. --- CONTRIBUTING.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index abbd91ddfe6f4..151a5cf8d2ad8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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 @@ -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 @@ -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.