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

Revert "url: support path for url.format" #303

Merged
merged 1 commit into from
Jan 12, 2015

Conversation

bnoordhuis
Copy link
Member

This reverts commit d312b6d.

Reverted for breaking npm install with git+ssh URLs.
See also the fix-up in commit a1e54d6.

Conflicts:
doc/api/url.markdown

Fixes: #295

R=@evanlucas or @rvagg, /cc @yorkie

@bnoordhuis
Copy link
Member Author

See also the fix-up in commit a1e54d6.

Maybe I should just leave that out, the fix-up was for another commit.

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

Unfortunately, lgtm. @bnoordhuis is the path forward with this to bring the feature back in and try and fix the underlying problem post 1.0.0? Can we give some guidance to @yorkie?

This reverts commit d312b6d.

Reverted for breaking `npm install` with git+ssh URLs.

Conflicts:
    doc/api/url.markdown

Fixes: nodejs#295
PR-URL: nodejs#303
Reviewed-By: Rod Vagg <[email protected]>
@bnoordhuis bnoordhuis merged commit 913addb into nodejs:v1.x Jan 12, 2015
@bnoordhuis bnoordhuis deleted the revert-d312b6d branch January 12, 2015 11:15
@bnoordhuis
Copy link
Member Author

I think the guidance should be that npm install <url> should keep working. :-) I have no opinion on bringing back the feature. If someone feels strongly, please open an issue.

@yorkie
Copy link
Contributor

yorkie commented Jan 12, 2015

Got it, thank you @rvagg @bnoordhuis i will try to add this feature again but with test covering the git+ssh case :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants