-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/request library #8
Feature/request library #8
Conversation
Master branch tests fail on work machine with
After this PR Unbox 3 passing (1s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
@tcoulter could you also review to test on Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - missed something. Mind making that addition? Then it should be good to merge.
lib/utils/unbox.js
Outdated
}; | ||
var req = https.request(options, function(r) { | ||
request(options, function(error, r) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@micahriggan actually one comment - would you mind adding a check for error
now that it's a new arg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
~ if (error) {
+ return reject(new Error("Error making request. Please check the format of the requested resource: " + options.uri));
+ } else if (r.statusCode == 404) {
Would something like this work?
The native nodejs https.request method doesn't follow proxy environment variables.
This is referenced by nodejs/node#8381
The request library does support these environment variables.
These environment variables are commonly used at the corporation I work for, so not supporting them is a barrier to entry for many corporate developers.
This should close
trufflesuite/truffle-init#1