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

crypto: add getIntOption function to reduce dupl #20247

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 24, 2018

This commit adds a getIntOption function to reduce the code duplicated
for getting the padding, and saltLength options.

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

This commit adds a getIntOption function to reduce the code duplicated
for getting the padding, and saltLength options.
@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Apr 24, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 24, 2018

@lpinca lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 24, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 26, 2018

node-test-commit-osx failure looks unrelated

console output:

Building addon /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/addons-napi/1_hello_world/
gyp info it worked if it ends with ok
gyp info using node-gyp@3.6.2
gyp info using node@10.0.0-pre | darwin | x64
gyp info chdir /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/addons-napi/1_hello_world/
gyp info spawn /usr/bin/python
gyp info spawn args [ '/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/deps/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/addons-napi/1_hello_world/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/deps/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010',
gyp info spawn args   '-Dnode_gyp_dir=/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/deps/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/$(Configuration)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/addons-napi/1_hello_world',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build', '--jobs', 2 ]
  CC(target) Release/obj.target/binding/binding.o
  SOLINK_MODULE(target) Release/binding.node
gyp info ok 

@danbev
Copy link
Contributor Author

danbev commented Apr 26, 2018

Landed in d4726d2.

@danbev danbev closed this Apr 26, 2018
@danbev danbev deleted the crypto_sig_js_get_options branch April 26, 2018 05:47
danbev added a commit that referenced this pull request Apr 26, 2018
This commit adds a getIntOption function to reduce the code duplicated
for getting the padding, and saltLength options.

PR-URL: #20247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
This commit adds a getIntOption function to reduce the code duplicated
for getting the padding, and saltLength options.

PR-URL: #20247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 8, 2018
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. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants