doc: prefer to sign commits under nodejs repository #57311

RafaelGSS · 2025-03-04T12:55:46Z

Although not required, I think adding this as a "recommended" is a good thing.

nodejs-github-bot · 2025-03-04T12:55:51Z

Review requested:

@nodejs/tsc

RafaelGSS · 2025-03-04T12:57:13Z

IIRC commit-queue does sign commits by default. But, in terms of creating releases, sometimes we forget to enforce this policy in backports, leading to some inconsistencies (as Antoine seen when we paired 😄)

legendecas · 2025-03-04T13:32:37Z

commit-queue does not sign if it is commit-queue-rebase, e.g. #57121

onboarding.md

Co-authored-by: Tobias Nießen <tniessen@tnie.de>

tniessen · 2025-03-04T13:34:38Z

FWIW, there have been multiple discussions about this in the past, and the benefit was generally considered marginal (see, for example, #15457, which would have made signing mandatory). That being said, I am not strictly opposed to this idea (and for a while I did sign my commits in this repository).

joyeecheung · 2025-03-04T14:56:45Z

I think this is only relevant for those who push directly to branches in the repository? For people who will probably never push directly to the repository (I guess most collaborators don't these days) it's not as relevant.

jasnell

If there isn't one already in this document, including a link to documentation on how to use commit.gpgsign would be helpful to newcomers.

aduh95 · 2025-03-04T19:39:10Z

Recommending signatures is certainly fine, but I don't think it would be relevant to most contributors. Signatures are only really useful for things that require trust (e.g. unreviewed backports, release promotion), and are nice-to-have everywhere else on nodejs/node, and IMO barely relevant for PRs (which is what most contributors will ever do).

RafaelGSS · 2025-03-05T18:24:05Z

Yes, this is non-relevant for most of the contributors but Node.js releasers or anyone that attempts to backport a commit manually. Still, it's an optional recommendation, I'm fine either way.

UlisesGascon

I think is a great recommendation, specially for releasers as we do some changed in the git history and our signatures can be verified (see)

Also worth mention that signature verification requires an extra step on GitHub commiter's side if we want to achieve the visual verification (ref):

aduh95 · 2025-03-05T21:52:59Z

onboarding.md

+  * It is recommended to sign all commits under the Node.js repository.
+    Run: `git config commit.gpgsign true` inside the `node` folder.


We should consider making this recommendation on the ONBOARDING.md of nodejs/Release, and maybe reword it to ensure we're not setting a bar too high for newcomers:

Suggested change

* It is recommended to sign all commits under the Node.js repository.

Run: `git config commit.gpgsign true` inside the `node` folder.

* Consider configuring git to sign all commits when working on the Node.js repository.

Run: `git config commit.gpgsign true` inside the `node` folder.

I fear that whatever the wording, people are going to run the command, and later they will be confused as their git commit fail because of the missing signing key.

Can it start with "if you have a GPG key set up locally that matches the email you use to contribute to Node.js" so that people who do not know what it is will skip it?

FWIW there are multiple methods of signing commits with git. GitHub supports (i.e. can displayed "Verified") GPG, SSH, or S/MIME signatures.

For the release team we've recommend GPG keys, but this documentation is aimed at a broader audience (collaborators).

RafaelGSS · 2025-03-11T13:52:49Z

Based on discussions on this PR, I think it would be more assertive if we suggest this comment in the Release onboarding section, although we don't have one. I can update https://github.com/nodejs/node/blob/main/doc/contributing/releases.md.

aduh95 · 2025-03-11T20:31:40Z

Based on discussions on this PR, I think it would be more assertive if we suggest this comment in the Release onboarding section, although we don't have one. I can update https://github.com/nodejs/node/blob/main/doc/contributing/releases.md.

We do have a list that onboardees have to follow in https://github.com/nodejs/Release/blob/main/GOVERNANCE.md#adding-new-releasers

nodejs-github-bot · 2025-03-17T08:59:15Z

Commit Queue failed

- Loading data for nodejs/node/pull/57311
✔  Done loading data for nodejs/node/pull/57311
----------------------------------- PR info ------------------------------------
Title      doc: prefer to sign commits under nodejs repository (#57311)
Author     Rafael Gonzaga <rafael.nunu@hotmail.com> (@RafaelGSS)
Branch     RafaelGSS:suggest-sign-commits -> nodejs:main
Labels     doc, author ready, commit-queue-squash
Commits    3
 - doc: prefer to sign commits under nodejs repository
 - Update onboarding.md
 - fixup! doc: prefer to sign commits under nodejs repository
Committers 2
 - RafaelGSS <rafael.nunu@hotmail.com>
 - GitHub <noreply@github.com>
PR-URL: https://github.com/nodejs/node/pull/57311
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/57311
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - fixup! doc: prefer to sign commits under nodejs repository
   ℹ  This PR was created on Tue, 04 Mar 2025 12:55:46 GMT
   ✔  Approvals: 5
   ✔  - Marco Ippolito (@marco-ippolito) (TSC): https://github.com/nodejs/node/pull/57311#pullrequestreview-2657348527
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/57311#pullrequestreview-2658025939
   ✔  - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/57311#pullrequestreview-2662015193
   ✔  - Ulises Gascón (@UlisesGascon): https://github.com/nodejs/node/pull/57311#pullrequestreview-2662286144
   ✔  - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/57311#pullrequestreview-2674699055
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/13895355131

nodejs-github-bot · 2025-03-17T09:10:29Z

Landed in bd8f2ba

Follow up nodejs/node#57311 (comment)

* Update GOVERNANCE.md Follow up nodejs/node#57311 (comment) * Update GOVERNANCE.md Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> --------- Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #57311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#57311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #57311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

doc: prefer to sign commits under nodejs repository

1277397

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 4, 2025

marco-ippolito approved these changes Mar 4, 2025

View reviewed changes

tniessen reviewed Mar 4, 2025

View reviewed changes

onboarding.md Outdated Show resolved Hide resolved

Update onboarding.md

c54e9e6

Co-authored-by: Tobias Nießen <tniessen@tnie.de>

RafaelGSS added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Mar 4, 2025

jasnell approved these changes Mar 4, 2025

View reviewed changes

This comment was marked as off-topic.

Sign in to view

MoLow approved these changes Mar 5, 2025

View reviewed changes

RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 5, 2025

UlisesGascon approved these changes Mar 5, 2025

View reviewed changes

aduh95 reviewed Mar 5, 2025

View reviewed changes

BridgeAR approved these changes Mar 11, 2025

View reviewed changes

fixup! doc: prefer to sign commits under nodejs repository

f0787fd

marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 17, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 17, 2025

marco-ippolito approved these changes Mar 17, 2025

View reviewed changes

marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Mar 17, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 17, 2025

nodejs-github-bot merged commit bd8f2ba into nodejs:main Mar 17, 2025
26 checks passed

RafaelGSS added a commit to nodejs/Release that referenced this pull request Mar 18, 2025

Update GOVERNANCE.md

1a65d12

Follow up nodejs/node#57311 (comment)

RafaelGSS mentioned this pull request Mar 18, 2025

Update GOVERNANCE.md nodejs/Release#1082

Merged

github-actions bot mentioned this pull request Mar 30, 2025

2025-04-01, Version 23.11.0 (Current) #57694

Merged

RafaelGSS mentioned this pull request Apr 8, 2025

v22.x-staging cherry-pick status #57799

Open

aduh95 mentioned this pull request Apr 11, 2025

2025-04-16, Version 22.15.0 'Jod' (LTS), @RafaelGSS #57840

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: prefer to sign commits under nodejs repository #57311

doc: prefer to sign commits under nodejs repository #57311

RafaelGSS commented Mar 4, 2025

nodejs-github-bot commented Mar 4, 2025

RafaelGSS commented Mar 4, 2025 •

edited

Loading

legendecas commented Mar 4, 2025

tniessen commented Mar 4, 2025

joyeecheung commented Mar 4, 2025

jasnell left a comment

This comment was marked as off-topic.

aduh95 commented Mar 4, 2025

RafaelGSS commented Mar 5, 2025 •

edited

Loading

UlisesGascon left a comment •

edited

Loading

aduh95 Mar 5, 2025

targos Mar 5, 2025

joyeecheung Mar 6, 2025 •

edited

Loading

richardlau Mar 6, 2025 •

edited

Loading

RafaelGSS commented Mar 11, 2025

aduh95 commented Mar 11, 2025

nodejs-github-bot commented Mar 17, 2025

nodejs-github-bot commented Mar 17, 2025

		* It is recommended to sign all commits under the Node.js repository.
		Run: `git config commit.gpgsign true` inside the `node` folder.

doc: prefer to sign commits under nodejs repository #57311

doc: prefer to sign commits under nodejs repository #57311

Conversation

RafaelGSS commented Mar 4, 2025

nodejs-github-bot commented Mar 4, 2025

RafaelGSS commented Mar 4, 2025 • edited Loading

legendecas commented Mar 4, 2025

tniessen commented Mar 4, 2025

joyeecheung commented Mar 4, 2025

jasnell left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

aduh95 commented Mar 4, 2025

RafaelGSS commented Mar 5, 2025 • edited Loading

UlisesGascon left a comment • edited Loading

Choose a reason for hiding this comment

aduh95 Mar 5, 2025

Choose a reason for hiding this comment

targos Mar 5, 2025

Choose a reason for hiding this comment

joyeecheung Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

richardlau Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

RafaelGSS commented Mar 11, 2025

aduh95 commented Mar 11, 2025

nodejs-github-bot commented Mar 17, 2025

nodejs-github-bot commented Mar 17, 2025

RafaelGSS commented Mar 4, 2025 •

edited

Loading

RafaelGSS commented Mar 5, 2025 •

edited

Loading

UlisesGascon left a comment •

edited

Loading

joyeecheung Mar 6, 2025 •

edited

Loading

richardlau Mar 6, 2025 •

edited

Loading