Commit 76c22f8
doc: edit Landing Pull Requests
Edit the Landing Pull Requests section of the Collaborators Guide for
brevity and clarity.
PR-URL: #26536
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>1 parent a32c749 commit 76c22f8
1 file changed
Lines changed: 19 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -388,32 +388,30 @@ The TSC should serve as the final arbiter where required. | |||
| 388 | 388 | ||
| 389 | 389 | ## Landing Pull Requests | |
| 390 | 390 | ||
| 391 | - 1. Avoid landing PRs that are assigned to someone else. Authors who wish to land | ||
| 392 | - their own PRs will self-assign them, or delegate to someone else. If in | ||
| 393 | - doubt, ask the assignee whether it is okay to land. | ||
| 391 | + 1. Avoid landing pull requests that have someone else as an assignee. Authors | ||
| 392 | + who wish to land their own pull requests will self-assign them. Sometimes, an | ||
| 393 | + author will delegate to someone else. If in doubt, ask the assignee whether | ||
| 394 | + it is okay to land. | ||
| 394 | 395 | 1. Never use GitHub's green ["Merge Pull Request"][] button. Reasons for not | |
| 395 | 396 | using the web interface button: | |
| 396 | 397 | * The "Create a merge commit" method will add an unnecessary merge commit. | |
| 397 | - * The "Squash and merge" method will add metadata (the PR #) to the commit | ||
| 398 | - title. If more than one author has contributed to the PR, squashing will | ||
| 399 | - only keep the most recent author. | ||
| 398 | + * The "Squash and merge" method will add metadata (the pull request #) to the | ||
| 399 | + commit title. If more than one author contributes to the pull request, | ||
| 400 | + squashing only keeps one author. | ||
| 400 | 401 | * The "Rebase and merge" method has no way of adding metadata to the commit. | |
| 401 | - 1. Make sure the CI is done and the result is green. If the CI is not green, | ||
| 402 | - check for flaky tests and infrastructure failures. Please check if those were | ||
| 403 | - already reported in the appropriate repository ([node][flaky tests] and | ||
| 404 | - [build](https://github.com/nodejs/build/issues)) or not and open new issues | ||
| 405 | - in case they are not. If no CI was run or the run is outdated because code | ||
| 406 | - was pushed after the last run, please first start a new CI and wait for the | ||
| 407 | - result. If no CI is required, please leave a comment in case none is already | ||
| 408 | - present. | ||
| 409 | - 1. Review the commit message to ensure that it adheres to the guidelines | ||
| 410 | - outlined in the [contributing][] guide. | ||
| 402 | + 1. Make sure CI is complete and green. If the CI is not green, check for | ||
| 403 | + unreliable tests and infrastructure failures. If there are not corresponding | ||
| 404 | + issues in the [node][unreliable tests] or | ||
| 405 | + [build](https://github.com/nodejs/build/issues) repositories, open new | ||
| 406 | + issues. Run a new CI any time someone pushes new code to the pull request. | ||
| 407 | + 1. Check that the commit message adheres to [commit message guidelines][]. | ||
| 411 | 408 | 1. Add all necessary [metadata](#metadata) to commit messages before landing. If | |
| 412 | 409 | you are unsure exactly how to format the commit messages, use the commit log | |
| 413 | 410 | as a reference. See [this commit][commit-example] as an example. | |
| 414 | 411 | ||
| 415 | - For PRs from first-time contributors, be [welcoming](#welcoming-first-time-contributors). | ||
| 416 | - Also, verify that their git settings are to their liking. | ||
| 412 | + For pull requests from first-time contributors, be | ||
| 413 | + [welcoming](#welcoming-first-time-contributors). Also, verify that their git | ||
| 414 | + settings are to their liking. | ||
| 417 | 415 | ||
| 418 | 416 | All commits should be self-contained, meaning every commit should pass all | |
| 419 | 417 | tests. This makes it much easier when bisecting to find a breaking change. | |
@@ -550,8 +548,7 @@ commit message for that commit. This is a good moment to fix incorrect | |||
| 550 | 548 | commit logs, ensure that they are properly formatted, and add | |
| 551 | 549 | `Reviewed-By` lines. | |
| 552 | 550 | ||
| 553 | - * The commit message text must conform to the | ||
| 554 | - [commit message guidelines](./doc/guides/contributing/pull-requests.md#commit-message-guidelines). | ||
| 551 | + * The commit message text must conform to the [commit message guidelines][]. | ||
| 555 | 552 | ||
| 556 | 553 | <a name="metadata"></a> | |
| 557 | 554 | * Modify the original commit message to include additional metadata regarding | |
@@ -793,12 +790,12 @@ If you cannot find who to cc for a file, `git shortlog -n -s <file>` may help. | |||
| 793 | 790 | [`--throw-deprecation`]: doc/api/cli.md#--throw-deprecation | |
| 794 | 791 | [`node-core-utils`]: https://github.com/nodejs/node-core-utils | |
| 795 | 792 | [backporting guide]: doc/guides/backporting-to-release-lines.md | |
| 796 | - [contributing]: ./doc/guides/contributing/pull-requests.md#commit-message-guidelines | ||
| 793 | + [commit message guidelines]: ./doc/guides/contributing/pull-requests.md#commit-message-guidelines | ||
| 797 | 794 | [commit-example]: https://github.com/nodejs/node/commit/b636ba8186 | |
| 798 | - [flaky tests]: https://github.com/nodejs/node/issues?q=is%3Aopen+is%3Aissue+label%3A%22CI+%2F+flaky+test%22y | ||
| 799 | 795 | [git-node]: https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md | |
| 800 | 796 | [git-node-metadata]: https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-metadata | |
| 801 | 797 | [git-username]: https://help.github.com/articles/setting-your-username-in-git/ | |
| 802 | 798 | [git-email]: https://help.github.com/articles/setting-your-commit-email-address-in-git/ | |
| 803 | 799 | [node-core-utils-credentials]: https://github.com/nodejs/node-core-utils#setting-up-credentials | |
| 804 | 800 | [node-core-utils-issues]: https://github.com/nodejs/node-core-utils/issues | |
| 801 | + [unreliable tests]: https://github.com/nodejs/node/issues?q=is%3Aopen+is%3Aissue+label%3A%22CI+%2F+flaky+test%22 | ||
0 commit comments