Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
url, test: add inspect to TupleOrigin#10039
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
url, test: add inspect to TupleOrigin #10039
Conversation
This adds a simple inspect function the the TupleOrigin class.
This adds tests for the newly added inspect function in the TupleOrigin class.
Trott commented Dec 2, 2016
addaleax left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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 with a suggestion, thanks!
lib/internal/url.js Outdated
| } | ||
| inspect(){ | ||
| return`TupleOrigin{ |
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 think we have spaces before the { in most cases here?
cjihrig left a comment
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 with @addaleax's nit addressed and a CI run.
jasnell commented Dec 2, 2016 via email
Lgtm …On Fri, Dec 2, 2016 at 11:19 AM Colin Ihrig ***@***.***> wrote: ***@***.**** approved this pull request. LGTM with @addaleax <https://github.com/addaleax>'s nit addressed and a CI run. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10039 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa2ecmm-yrL9WiUggst-DNXQrt8VTw_ks5rEFMMgaJpZM4LBsld> . |
captainsafia commented Dec 2, 2016
Nit addressed. |
gibfahn commented Dec 2, 2016
addaleax commented Dec 2, 2016
@captainsafia Looks like you need to update the test files too? :) |
addaleax commented Dec 2, 2016
Thanks for being so quick to update! Here’s a new CI: https://ci.nodejs.org/job/node-test-commit/6348/ |
captainsafia commented Dec 2, 2016
@addaleax: Whoops. Excuse my merge-eager fingers. Fixed now. 😊 |
addaleax commented Dec 5, 2016
Landed in 8831732, thanks for the contribution! |
This adds a simple inspect function the the TupleOrigin class. This adds tests for the newly added inspect function in the TupleOrigin class. PR-URL: #10039 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
jasnell commented Dec 5, 2016
Thank you for this @captainsafia ... this one has been on my list of todo's for remaining items on the new |
This adds a simple inspect function the the TupleOrigin class. This adds tests for the newly added inspect function in the TupleOrigin class. PR-URL: nodejs#10039 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
This adds a simple inspect function the the TupleOrigin class. This adds tests for the newly added inspect function in the TupleOrigin class. PR-URL: #10039 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Notable changes SEMVER-MINOR - url: - add inspect function to TupleOrigin (Safia Abdalla) #10039 - crypto: - allow adding extra certs to well-known CAs (Sam Roberts) #9139 SEMVER-PATCH - buffer: - fix single-character string filling (Anna Henningsen) #9837 - handle UCS2 .fill() properly on BE (Anna Henningsen) #9837 - url: - including base argument in originFor (joyeecheung) #10021 - improve URLSearchParams spec compliance (Timothy Gu) #9484 - http: - remove stale timeout listeners (Karl Böhlmark) #9440 - build: - fix node_g target (Daniel Bevenius) #10153 - fs: - remove unused argument from copyObject() (Ethan Arrowood) #10041 - timers: - fix handling of cleared immediates (hveldstra) #9759 PR-URL: #10277
Notable changes: * **crypto**: - Allow adding extra certificates to well-known CAs. (Sam Roberts) [#9139](#9139) * **buffer**: - Fix single-character string filling. (Anna Henningsen) [#9837](#9837) - Handle UCS2 `.fill()` properly on BE. (Anna Henningsen) [#9837](#9837) * **url**: - Add inspect function to TupleOrigin. (Safia Abdalla) [#10039](#10039) - Including base argument in originFor. (joyeecheung) [#10021](#10021) - Improve URLSearchParams spec compliance. (Timothy Gu) [#9484](#9484) * **http**: - Remove stale timeout listeners. (Karl Böhlmark) [#9440](#9440) * **build**: - Fix node_g target. (Daniel Bevenius) [#10153](#10153) * **fs**: - Remove unused argument from copyObject(). (EthanArrowood) [#10041](#10041) * **timers**: - Fix handling of cleared immediates. (hveldstra) [#9759](#9759) * **src**: - Add wrapper for process.emitWarning(). (SamRoberts) [#9139](#9139) - Fix string format mistake for 32 bit node.(Alex Newman) [#10082](#10082)
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
url, test
Description of change
This PR adds code and tests for an inspect function on the
TupleOriginobject.