Add --cafile command line option.#837
Add --cafile command line option.#837bnoordhuis merged 2 commits intonodejs:masterfrom bnoordhuis:cafile
Conversation
There was a problem hiding this comment.
taking a cb option that's only used when erroring is a bit gross, perhaps it'd closer to idiomatic if the thrown error bubbled up and the try/catch was done at the download() call?
|
A bit smelly, I understand that it's hard to do much better without a more heavy refactor. Things I don't like:
A begrudging lgtm since I understand the challenges of doing a better job, I would prefer the Yet another refactor for the future I suppose. |
|
I added a commit that lets the exception bubble up. I'm a bit on the fence on whether it's an improvement; let me know what you think. |
|
I'm happy with the clarity of the changes, but generally unhappy with the structure of the call chain but that's a matter for another day! lgtm |
Move the function around so it can be tested and add a regression test. As a policy vs. mechanism thing, change the control flow to handle exceptions at the call site, not inside the download function. PR-URL: #837 Reviewed-By: Rod Vagg <[email protected]>
Add an option for overriding the default CA chain that is used when downloading the tarball. This matches the npm option of the same name and gets implicitly passed through the `npm_config_cafile` environment variable. Fixes: #695 PR-URL: #837 Reviewed-By: Rod Vagg <[email protected]>
…o "undefined" cafile Refs: nodejs/node-gyp#837, nodejs/node-gyp#844 Failed job: https://ci.appveyor.com/project/brianreavis/node-gdal/build/job/n9u9mr7a sfrlbs55
…o "undefined" cafile Refs: nodejs/node-gyp#837, nodejs/node-gyp#844 Failed job: https://ci.appveyor.com/project/brianreavis/node-gdal/build/job/n9u9mr7a sfrlbs55
2 participants
R=@rvagg?