Skip to content
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

doc: url.format - true slash postfix behaviour #4119

Conversation

claudiorodriguez
Copy link
Contributor

Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: #3361
Also ccing to #4101

Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: nodejs#3361
@mscdex mscdex added doc Issues and PRs related to the documentations. url Issues and PRs related to the legacy built-in url module. labels Dec 2, 2015
@silverwind
Copy link
Contributor

I don't think we should document broken behaviour, see #3361 (comment).

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

Well... the current behavior ought to be documented and if it's broken, it ought be fixed ;-) Until it's fixed, it definitely should be documented tho

@silverwind
Copy link
Contributor

Right, you have a point. We can land this and then possible follow up with a breaking change that removes it again.

LGTM

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

LGTM

@claudiorodriguez
Copy link
Contributor Author

@silverwind once this lands I'll probably submit another PR doing that, will have to add a few tests to test-url (which has gotten quite large, probably a refactor into separate files might be warranted too)

@JungMinu
Copy link
Member

JungMinu commented Dec 5, 2015

LGTM

@JungMinu
Copy link
Member

JungMinu commented Dec 5, 2015

May I land this? :)

@silverwind
Copy link
Contributor

@JungMinu go ahead.

JungMinu pushed a commit that referenced this pull request Dec 6, 2015
Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: #3361

PR-URL: #4119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@JungMinu
Copy link
Member

JungMinu commented Dec 6, 2015

Thanks, landed in 2a29b70

@JungMinu JungMinu closed this Dec 6, 2015
rvagg pushed a commit that referenced this pull request Dec 8, 2015
Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: #3361

PR-URL: #4119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@rvagg rvagg mentioned this pull request Dec 17, 2015
MylesBorins pushed a commit that referenced this pull request Dec 29, 2015
Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: #3361

PR-URL: #4119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: #3361

PR-URL: #4119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Change url.format's references to slash postfixing to reflect
true behaviour (it only automatically postfixes slashes to the
slashedProtocols when host is present).

Fixes: nodejs#3361

PR-URL: nodejs#4119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

url.format does not postfix slashes to protocol but doc pretend it should by default
6 participants