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

src: refactor ECDHBitsJob signature #55610

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

panva
Copy link
Member

@panva panva commented Oct 30, 2024

removes use of GetOKPCurveFromName in ECDHBitsJob and moves GetOKPCurveFromName closer to its only remaining invocation.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. 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 Oct 30, 2024
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.

Project coverage is 88.44%. Comparing base (6dea41d) to head (a445d57).
Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_ec.cc 20.00% 0 Missing and 4 partials ⚠️
src/crypto/crypto_keys.cc 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #55610   +/-   ##
=======================================
  Coverage   88.43%   88.44%           
=======================================
  Files         654      654           
  Lines      187699   187694    -5     
  Branches    36125    36123    -2     
=======================================
+ Hits       165995   165997    +2     
+ Misses      14948    14946    -2     
+ Partials     6756     6751    -5     
Files with missing lines Coverage Δ
lib/internal/crypto/diffiehellman.js 97.52% <ø> (-0.01%) ⬇️
src/crypto/crypto_ec.h 9.09% <ø> (ø)
src/crypto/crypto_keys.h 58.18% <ø> (ø)
src/crypto/crypto_keys.cc 73.45% <83.33%> (+0.39%) ⬆️
src/crypto/crypto_ec.cc 67.40% <20.00%> (-0.48%) ⬇️

... and 22 files with indirect coverage changes

@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 31, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@panva panva added crypto Issues and PRs related to the crypto subsystem. webcrypto labels Nov 2, 2024
@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2024
@nodejs-github-bot nodejs-github-bot merged commit 4bf5731 into nodejs:main Nov 2, 2024
71 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4bf5731

@panva panva deleted the ecdh-cleanup branch November 2, 2024 12:37
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
PR-URL: nodejs#55610
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 5, 2024
PR-URL: #55610
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
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. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. webcrypto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants