-
Notifications
You must be signed in to change notification settings - Fork 141
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
Fix bugs for 1.0.1 #353
Fix bugs for 1.0.1 #353
Conversation
AutoCollection/HttpDependencies.ts
Outdated
}; | ||
} | ||
https.request = (options, ...requestArgs: any[]) => { | ||
const request: http.ClientRequest = originalHttpsRequest.call( |
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.
isn't it exactly the same as for http.request? Would it make sense to move it to a method and just pass originalHttpsRequest/originalRequest object?
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.
Nope, the base functions take different numbers of arguments 😄
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.
I do really dislike the duplication though.. For what its worth, it was already duplicated like this before the changes
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.
Ok, let's leave it as it is.
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.
Actually scratch that, I think this can be cleanly de-duplicated. Will change
Address #352, #351, #349, #348 and prepare a 1.0.1 release.