FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: move some X509Certificate stuff to ncrypto by jasnell · Pull Request #54241 · nodejs/node · GitHub

/ node Public

src: move some X509Certificate stuff to ncrypto - #54241

Closed
jasnell wants to merge 10 commits into
nodejs:mainfrom
jasnell:use-ncrypto-moar
ConversationCommits (10)ChecksFiles changed
Closed

src: move some X509Certificate stuff to ncrypto#54241
jasnell wants to merge 10 commits into
nodejs:mainfrom
jasnell:use-ncrypto-moar

Conversation

jasnell commented Aug 7, 2024

Copy link
Copy Markdown
Member

Starting working on moving the core bits of X509Certificate to ncrypto... this is incomplete but it's a big set of changes so I'm splitting them up in to multiple chunks.

jasnell requested review from anonrig and tniessen August 7, 2024 04:58

nodejs-github-bot commented Aug 7, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/security-wg

jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024
nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 7, 2024
jasnell added crypto Issues and PRs related to the crypto subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. dependencies Pull requests that update a dependency file. labels Aug 7, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
anonrig reviewed Aug 7, 2024
View reviewed changes
Comment thread deps/ncrypto/engine.cc
Comment thread deps/ncrypto/ncrypto.cc
Comment thread src/crypto/crypto_x509.cc Outdated
Comment thread src/crypto/crypto_x509.cc
Comment thread src/crypto/crypto_x509.cc
anonrig reviewed Aug 7, 2024
View reviewed changes
Comment thread src/crypto/crypto_x509.h Outdated

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented Aug 7, 2024 •
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60953/ 💛

jasnell added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed needs-ci PRs that need a full CI run. labels Aug 8, 2024
anonrig requested review from benjamingr and mcollina August 12, 2024 15:13
mcollina approved these changes Aug 12, 2024
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM as long as CI passes

jasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 12, 2024
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 Aug 12, 2024

nodejs-github-bot commented Aug 12, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/54241
✔  Done loading data for nodejs/node/pull/54241
----------------------------------- PR info ------------------------------------
Title      src: move some X509Certificate stuff to ncrypto (#54241)
Author     James M Snell <jasnell@gmail.com> (@jasnell)
Branch     jasnell:use-ncrypto-moar -> nodejs:main
Labels     crypto, c++, lib / src, author ready, dependencies
Commits    10
 - src: move some X509Certificate stuff to ncrypto
 - src: shift more crypto X509 to ncrypto
 - src: shift more x509 to ncrypto
 - src: shift more x509 to ncrypto
 - src: shift even more x509 to ncrypto
 - src: shift more x509 to ncrypto
 - src: fixup crypto linting after x509 changes
 - src: fixup includes after x509 changes
 - src: fixups after code review
 - src: address nits
Committers 1
 - James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/54241
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54241
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 07 Aug 2024 04:58:43 GMT
   ✔  Approvals: 2
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/54241#pullrequestreview-2225624836
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/54241#pullrequestreview-2233450678
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-08-07T22:13:52Z: https://ci.nodejs.org/job/node-test-pull-request/60953/
- Querying data for job/node-test-pull-request/60953/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 54241
From https://github.com/nodejs/node
 * branch                  refs/pull/54241/merge -> FETCH_HEAD
✔  Fetched commits as 624db509520a..083bad70e707
--------------------------------------------------------------------------------
[main 4aa5f71b56] src: move some X509Certificate stuff to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Mon Aug 5 16:24:51 2024 -0700
 2 files changed, 43 insertions(+), 1 deletion(-)
[main 8f81e293dd] src: shift more crypto X509 to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 19:28:20 2024 -0700
 4 files changed, 103 insertions(+), 20 deletions(-)
[main 9b78f096ba] src: shift more x509 to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 20:43:10 2024 -0700
 4 files changed, 210 insertions(+), 69 deletions(-)
[main 0e14dbdf2d] src: shift more x509 to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 21:12:59 2024 -0700
 6 files changed, 125 insertions(+), 85 deletions(-)
[main df19fced42] src: shift even more x509 to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 21:39:11 2024 -0700
 4 files changed, 138 insertions(+), 96 deletions(-)
[main 27eb278981] src: shift more x509 to ncrypto
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 21:54:26 2024 -0700
 4 files changed, 48 insertions(+), 49 deletions(-)
[main 6d166ebcb3] src: fixup crypto linting after x509 changes
 Author: James M Snell <jasnell@gmail.com>
 Date: Tue Aug 6 21:55:02 2024 -0700
 2 files changed, 45 insertions(+), 45 deletions(-)
[main b268532cfd] src: fixup includes after x509 changes
 Author: James M Snell <jasnell@gmail.com>
 Date: Wed Aug 7 06:59:26 2024 -0700
 4 files changed, 9 insertions(+), 14 deletions(-)
[main 947f98a9f2] src: fixups after code review
 Author: James M Snell <jasnell@gmail.com>
 Date: Wed Aug 7 07:52:31 2024 -0700
 1 file changed, 1 insertion(+), 3 deletions(-)
[main 8272776189] src: address nits
 Author: James M Snell <jasnell@gmail.com>
 Date: Wed Aug 7 08:43:42 2024 -0700
 2 files changed, 4 insertions(+), 4 deletions(-)
   ✔  Patches applied
There are 10 commits in the PR. Attempting autorebase.
Rebasing (2/20)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: move some X509Certificate stuff to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 138db0c245] src: move some X509Certificate stuff to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Mon Aug 5 16:24:51 2024 -0700

2 files changed, 43 insertions(+), 1 deletion(-)

Rebasing (3/20)

Rebasing (4/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: shift more crypto X509 to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD d866d08687] src: shift more crypto X509 to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 19:28:20 2024 -0700

4 files changed, 103 insertions(+), 20 deletions(-)

Rebasing (5/20)

Rebasing (6/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: shift more x509 to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD b95758585b] src: shift more x509 to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 20:43:10 2024 -0700

4 files changed, 210 insertions(+), 69 deletions(-)

Rebasing (7/20)

Rebasing (8/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: shift more x509 to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 31206a134f] src: shift more x509 to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 21:12:59 2024 -0700

6 files changed, 125 insertions(+), 85 deletions(-)

Rebasing (9/20)

Rebasing (10/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: shift even more x509 to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 6ea876450c] src: shift even more x509 to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 21:39:11 2024 -0700

4 files changed, 138 insertions(+), 96 deletions(-)

Rebasing (11/20)

Rebasing (12/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: shift more x509 to ncrypto


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 2cabbd704b] src: shift more x509 to ncrypto

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 21:54:26 2024 -0700

4 files changed, 48 insertions(+), 49 deletions(-)

Rebasing (13/20)

Rebasing (14/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: fixup crypto linting after x509 changes


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 5eecf9d324] src: fixup crypto linting after x509 changes

Author: James M Snell <jasnell@gmail.com>

Date: Tue Aug 6 21:55:02 2024 -0700

2 files changed, 45 insertions(+), 45 deletions(-)

Rebasing (15/20)

Rebasing (16/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: fixup includes after x509 changes


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD 1e73f13ae5] src: fixup includes after x509 changes

Author: James M Snell <jasnell@gmail.com>

Date: Wed Aug 7 06:59:26 2024 -0700

4 files changed, 9 insertions(+), 14 deletions(-)

Rebasing (17/20)

Rebasing (18/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: fixups after code review


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD d5c10d06ca] src: fixups after code review

Author: James M Snell <jasnell@gmail.com>

Date: Wed Aug 7 07:52:31 2024 -0700

1 file changed, 1 insertion(+), 3 deletions(-)

Rebasing (19/20)

Rebasing (20/20)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: address nits


PR-URL: #54241

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>


[detached HEAD eed729e2b2] src: address nits

Author: James M Snell <jasnell@gmail.com>

Date: Wed Aug 7 08:43:42 2024 -0700

2 files changed, 4 insertions(+), 4 deletions(-)


Successfully rebased and updated refs/heads/main.


ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.
https://github.com/nodejs/node/actions/runs/10357382875

jasnell added a commit that referenced this pull request Aug 12, 2024
src: move some X509Certificate stuff to ncrypto
5f230d2 
PR-URL: #54241
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell commented Aug 12, 2024

Copy link
Copy Markdown
Member Author

Landed in 5f230d2

jasnell closed this Aug 12, 2024
jasnell removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 12, 2024
targos pushed a commit that referenced this pull request Aug 14, 2024
src: move some X509Certificate stuff to ncrypto
b6927dd 
PR-URL: #54241
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
RafaelGSS mentioned this pull request Aug 19, 2024
Merged
npaun pushed a commit to npaun/ncrypto that referenced this pull request Sep 18, 2025
src: move some X509Certificate stuff to ncrypto
8218bd2 
PR-URL: nodejs/node#54241
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
anonrig anonrig approved these changes
tniessen Awaiting requested review from tniessen
benjamingr Awaiting requested review from benjamingr
+1 more reviewer
mertcanaltin mertcanaltin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. dependencies Pull requests that update a dependency file. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL