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.

Sign up for GitHub

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

Jump to bottom

doc: additional note in README(.md) informing users that it is advise… #32655

Closed
wants to merge 4 commits into from
Closed

doc: additional note in README(.md) informing users that it is advise… #32655

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9bd6618
doc: additional note in README(.md) informing users that it is advise…
haqer1 Apr 4, 2020
2e103ed
doc: apply suggestion from code review
haqer1 Apr 9, 2020
945cce5
doc: additional note in README(.md) informing users that it is advise…
haqer1 Oct 28, 2020
69d81a9
doc: additional note in README(.md) informing users that it is advise…
haqer1 Oct 28, 2020
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
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ GPG keys used to sign Node.js releases:
* **Shelley Vohr** <shelley.vohr@gmail.com>
`B9E2F5981AA6E0CD28160D9FF13993A75599653C`

To import the full set of trusted release keys:
To avoid nuances involved in verification of a sub-key possibly used to sign a
release, import the full set of trusted release keys:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

To import the full set of trusted release keys (including subkeys used to sign releases):

I think the current test has a bit too much editorialization. I think suggested text above accomplishes the intention while keeping things succinct.

LMK if you have alternative suggestions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

To import the full set of trusted release keys (including subkeys used to sign releases):

IMHO, there is a need to mention something like "nuance"s or "pitfall"s, to make sure that users get an idea that if they choose to not import all the keys following (assuming #32654 is not going to land), they might (well) run into something they aren't expecting. My motivation was to prevent confusion & misuse of time (which happened to me).
So between this suggestion & the current version, i'd prefer the current version (because it's more informative, etc.).

P.S. It seems there is an agreement on a need for change, & hopefully there will be an agreement on the wording w/o spending too much time on it. For me, the current wording committed is better that the status quo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @Trott @MylesBorins

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stand by my original review. I think the current text is too editorialized and prefer the proposed text I suggested. I'm open to reviewing alternative suggestions


```shell
gpg --keyserver pool.sks-keyservers.net --recv-keys 4ED778F539E3634C779C87C6D7062848A1AB005C
Expand Down