@@ -365,60 +365,45 @@ The TSC should serve as the final arbiter where required.
365365 * If more than one author has contributed to the PR, keep the most recent
366366 author when squashing.
367367
368- Always modify the original commit message to include additional meta
369- information regarding the change process:
370-
371- - A ` PR-URL: ` line that references the * full* GitHub URL of the original
372- pull request being merged so it's easy to trace a commit back to the
373- conversation that led up to that change.
374- - A ` Fixes: X ` line, where _ X_ either includes the * full* GitHub URL
375- for an issue, and/or the hash and commit message if the commit fixes
376- a bug in a previous commit. Multiple ` Fixes: ` lines may be added if
377- appropriate.
378- - A ` Refs: ` line referencing a URL for any relevant background.
379- - A ` Reviewed-By: Name <email> ` line for yourself and any
380- other Collaborators who have reviewed the change.
381- - Useful for @mentions / contact list if something goes wrong in the PR.
382- - Protects against the assumption that GitHub will be around forever.
383-
384368Review the commit message to ensure that it adheres to the guidelines outlined
385369in the [ contributing] ( ./CONTRIBUTING.md#commit-message-guidelines ) guide.
386370
371+ Add all necessary [ metadata] ( #metadata ) to commit messages before landing.
372+
387373See the commit log for examples such as
388374[ this one] ( https://github.com/nodejs/node/commit/b636ba8186 ) if unsure
389375exactly how to format your commit messages.
390376
391377Additionally:
392378- Double check PRs to make sure the person's _ full name_ and email
393379 address are correct before merging.
394- - Except when updating dependencies, all commits should be self
395- contained (meaning every commit should pass all tests). This makes
396- it much easier when bisecting to find a breaking change.
380+ - All commits should be self-contained (meaning every commit should pass all
381+ tests). This makes it much easier when bisecting to find a breaking change.
397382
398383### Technical HOWTO
399384
400- Clear any ` am ` /` rebase ` that may already be underway.
385+ Clear any ` am ` /` rebase ` that may already be underway:
401386
402387``` text
403388$ git am --abort
404389$ git rebase --abort
405390```
406391
407- Checkout proper target branch
392+ Checkout proper target branch:
408393
409394``` text
410395$ git checkout master
411396```
412397
413398Update the tree (assumes your repo is set up as detailed in
414- [ CONTRIBUTING.md] ( CONTRIBUTING.md#step-1-fork ) )
399+ [ CONTRIBUTING.md] ( CONTRIBUTING.md#step-1-fork ) ):
415400
416401``` text
417402$ git fetch upstream
418403$ git merge --ff-only upstream/master
419404```
420405
421- Apply external patches
406+ Apply external patches:
422407
423408``` text
424409$ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
@@ -436,21 +421,19 @@ against the original PR carefully and build/test on at least one platform
436421before landing. If the 3-way merge fails, then it is most likely that a conflicting
437422PR has landed since the CI run and you will have to ask the author to rebase.
438423
439- Check and re-review the changes
424+ Check and re-review the changes:
440425
441426``` text
442427$ git diff upstream/master
443428```
444429
445- Check number of commits and commit messages
430+ Check number of commits and commit messages:
446431
447432``` text
448433$ git log upstream/master...master
449434```
450435
451- If there are multiple commits that relate to the same feature or
452- one with a feature and separate with a test for that feature,
453- you'll need to use ` squash ` or ` fixup ` :
436+ Squash commits and add metadata:
454437
455438``` text
456439$ git rebase -i upstream/master
@@ -506,9 +489,29 @@ Save the file and close the editor. You'll be asked to enter a new
506489commit message for that commit. This is a good moment to fix incorrect
507490commit logs, ensure that they are properly formatted, and add
508491` Reviewed-By ` lines.
492+
509493* The commit message text must conform to the
510494[ commit message guidelines] ( ./CONTRIBUTING.md#commit-message-guidelines ) .
511495
496+ <a name =" metadata " ></a >
497+ * Modify the original commit message to include additional metadata regarding
498+ the change process. (If you use Chrome or Edge, [ ` node-review ` ] [ ] fetches
499+ the metadata for you.)
500+
501+ * Required: A ` PR-URL: ` line that references the * full* GitHub URL of the
502+ original pull request being merged so it's easy to trace a commit back to
503+ the conversation that led up to that change.
504+ * Optional: A ` Fixes: X ` line, where _ X_ either includes the * full* GitHub URL
505+ for an issue, and/or the hash and commit message if the commit fixes
506+ a bug in a previous commit. Multiple ` Fixes: ` lines may be added if
507+ appropriate.
508+ * Optional: One or more ` Refs: ` lines referencing a URL for any relevant
509+ background.
510+ * Required: A ` Reviewed-By: Name <email> ` line for yourself and any
511+ other Collaborators who have reviewed the change.
512+ * Useful for @mentions / contact list if something goes wrong in the PR.
513+ * Protects against the assumption that GitHub will be around forever.
514+
512515Run tests (` make -j4 test ` or ` vcbuild test ` ). Even though there was a
513516successful continuous integration run, other changes may have landed on master
514517since then, so running the tests one last time locally is a good practice.
@@ -672,3 +675,4 @@ LTS working group and the Release team.
672675[ Stability Index ] : doc/api/documentation.md#stability-index
673676[ Enhancement Proposal ] : https://github.com/nodejs/node-eps
674677[ git-username ] : https://help.github.com/articles/setting-your-username-in-git/
678+ [ `node-review` ] : https://github.com/evanlucas/node-review
0 commit comments