This is just so that this repo can be reviewed#1
This is just so that this repo can be reviewed#1jjhoughton wants to merge 25 commits intoempty-branchfrom
Conversation
|
@jfmesse would be a good person to review this |
test.js
Outdated
| `; | ||
|
|
||
| try { | ||
| console.log("My rsa private key is", modulus.RSAPrivateKey(rsakey)); |
There was a problem hiding this comment.
i think you should add a test in this file to make sure you get the same as using the command line
There was a problem hiding this comment.
maybe add a benchmark test comparison too
package.json
Outdated
| "private": true, | ||
| "gypfile": true, | ||
| "scripts": { | ||
| "start": "node-gyp configure build && node module.js", |
There was a problem hiding this comment.
its weird to have a start script for a lib like this
There was a problem hiding this comment.
It doesn't work it needs deleting
| char *buf; | ||
| napi_status status = napi_ok; | ||
|
|
||
| BIO *bio; |
There was a problem hiding this comment.
i think you should link to the openssl docs were this came from
| } | ||
| if (napi_get_value_string_utf8 (env, argv[0], NULL, 0, &size) != napi_ok) | ||
| { | ||
| napi_throw_error (env, NULL, "Failed to parse string"); |
There was a problem hiding this comment.
can you add more detail in all these errors?
| } | ||
| if (argc < 1) | ||
| { | ||
| napi_throw_error (env, NULL, "This function requires one argument"); |
There was a problem hiding this comment.
shouldn't this say what the argument is supposed to be
| if (napi_get_value_string_utf8 (env, argv[0], buf, | ||
| size, &size) != napi_ok) | ||
| { | ||
| napi_throw_error (env, NULL, "Failed to parse string"); |
There was a problem hiding this comment.
this is the same error message as above, id change it so you know where it wrong
There was a problem hiding this comment.
It's because it's the same function as above. The one above just checks the size of the string and the second copies the string to a c string
test.js
Outdated
| @@ -0,0 +1,58 @@ | |||
| const modulus = require("."); | |||
There was a problem hiding this comment.
id probably rename this to lib or something
There was a problem hiding this comment.
Yep i changed the name of the project at some point
package.json
Outdated
| "gypfile": true, | ||
| "scripts": { | ||
| "start": "node-gyp configure build && node module.js", | ||
| "test": "node test.js" |
There was a problem hiding this comment.
id have thought you'd use a testing lib to make sure your code meets some expectations, otherwise its jut ascript you run
| "version": "0.1.0", | ||
| "main": "main.js", | ||
| "private": true, | ||
| "gypfile": true, |
There was a problem hiding this comment.
Makes it compile when you run yarn
moxonj
left a comment
There was a problem hiding this comment.
i get this error building on woof 2
[ripjar@wf-dev2 node-openssl]$ /apps/libs/node-8/bin/npm install
node-openssl@0.1.0 install /home/ripjar/github-ripjar/node-openssl
node-gyp rebuild
make: Entering directory /home/ripjar/github-ripjar/node-openssl/build' CC(target) Release/obj.target/node_openssl/main.o ../main.c:20:22: fatal error: node_api.h: No such file or directory #include <node_api.h> ^ compilation terminated. make: *** [Release/obj.target/node_openssl/main.o] Error 1 make: Leaving directory /home/ripjar/github-ripjar/node-openssl/build'
gyp ERR! build error
gyp ERR! stack Error: make failed with exit code: 2
gyp ERR! stack at ChildProcess.onExit (/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack at emitTwo (events.js:106:13)
gyp ERR! stack at ChildProcess.emit (events.js:191:7)
gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:219:12)
gyp ERR! System Linux 3.10.0-693.17.1.el7.x86_64
gyp ERR! command "/apps/libs/node-6.13.1/bin/node" "/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/ripjar/github-ripjar/node-openssl
gyp ERR! node -v v6.13.1
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! node-openssl@0.1.0 install: node-gyp rebuild
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the node-openssl@0.1.0 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?
npm ERR! A complete log of this run can be found in:
npm ERR! /home/ripjar/.npm/_logs/2019-07-09T17_03_40_404Z-debug.log
|
It compiles alright you just need to change the PATH to use node 8 like so |
|
|
||
| if (napi_create_function (env, NULL, 0, rsa_priv_key, | ||
| NULL, &rsa_fn) != napi_ok) | ||
| napi_throw_error (env, NULL, "Unable to wrap native rsa function"); |
There was a problem hiding this comment.
These should have return statements all of them. I don't think it stop when it hits an error
|
Drum thinks i should add some comments describing what section is what. I agree |
Signed-off-by: James Moxon <james.moxon@ripjar.com> Signed-off-by: Joshua Houghton <joshua.houghton@ripjar.com>
merge master in to develop
use middleman workflows
https://github.com/ripjar/workflow-server/pull/182/files
https://ripjar.atlassian.net/browse/LP-2254