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.

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

[v8.x backport] doc: add and unify return statements in crypto.md #22870

Closed
wants to merge 1 commit into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Sep 15, 2018

Original PR #19853
Fixes #22853

Conform return statements to the style guide and tool parsers.

Also bring back a description fragment
that seems to be erroneously deleted in
1e07acd

PR-URL: #19853
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Rich Trott rtrott@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Gibson Fahnestock gibfahn@gmail.com
Reviewed-By: Trivikram Kamat trivikr.dev@gmail.com
Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
Reviewed-By: Tobias Nießen tniessen@tnie.de

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Conform return statements to the style guide and tool parsers.

Also bring back a description fragment
that seems to be erroneously deleted in
nodejs@1e07acd

PR-URL: nodejs#19853
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. v8.x labels Sep 15, 2018
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Thank you!

@vsemozhetbyt
Copy link
Contributor

cc @nodejs/crypto

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 15, 2018
@trivikr
Copy link
Member Author

trivikr commented Sep 25, 2018

I'm getting the following error while trying to land on v8.x-staging:

$ git node land 22870
⠹ Getting commits from nodejs/node/pull/22870{ Error: GraphQL request Error: Bad credentials
    at Request.query (/home/trivikr/.nvm/versions/node/v10.11.0/lib/node_modules/node-core-utils/lib/request.js:89:19)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  data:
   { variables: { after: null, prid: 22870, owner: 'nodejs', repo: 'node' } } }

Any idea what I could be doing wrong?

MylesBorins pushed a commit that referenced this pull request Sep 25, 2018
Conform return statements to the style guide and tool parsers.

Also bring back a description fragment
that seems to be erroneously deleted in
1e07acd

Backport-PR-URL: #22870
PR-URL: #19853
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins
Copy link
Contributor

@trivikr only members of the LTS team can land on LTS branches. It could also be due to the commit already having meta data and only needing the backport metadata

landed in 0340dd8

@richardlau
Copy link
Member

@trivikr only members of the LTS team can land on LTS branches. It could also be due to the commit already having meta data and only needing the backport metadata

https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#how-are-lts-branches-managed says:

Any Collaborator may land commits into a staging branch, but only the release team should land commits into the LTS branch while preparing a new LTS release.

@trivikr trivikr deleted the backport-19853-to-v8.x branch September 25, 2018 21:18
@trivikr
Copy link
Member Author

trivikr commented Sep 25, 2018

Thanks @MylesBorins @richardlau
Can the message be improved in node-core-utils? It does say bad credentials which is good enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants