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: update curl usage in COLLABORATOR_GUIDE #1382

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link
Contributor

Looks like Github now redirects the patch urls from

https://github.com/iojs/io.js/pull/1382.patch

to

https://patch-diff.githubusercontent.com/raw/iojs/io.js/pull/1382.patch

so curl now needs to follow that redirect.

@silverwind silverwind added the doc Issues and PRs related to the documentations. label Apr 9, 2015
@bnoordhuis
Copy link
Member

LGTM but maybe add one or two lines to the commit log explaining the change. The idea is that when looking at the history, you don't have to look at the diff to figure out what the actual change was.

@silverwind
Copy link
Contributor Author

Thanks, will do.

silverwind added a commit that referenced this pull request Apr 9, 2015
Github now redirects the patch urls hosted on github.com, making it
necessary to enable curl redirect handling to obtain patches in the
merge process.

Example (before and after):
https://github.com/iojs/io.js/pull/1382.patch
https://patch-diff.githubusercontent.com/raw/iojs/io.js/pull/1382.patch

PR-URL: #1382
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@silverwind
Copy link
Contributor Author

Landed in b464d46

@silverwind silverwind closed this Apr 9, 2015
@petkaantonov
Copy link
Contributor

btw there is apply-pr utility I wrote that has been pretty handy: https://github.com/petkaantonov/apply-pr

Applying pr is like: apply-pr --remote=upstream 1234 where 1234 is the pr id :)

@bnoordhuis
Copy link
Member

Another option is to add fetch = +refs/pull/*/head:refs/pr/* to your .git/config and then you can merge PRs with git merge refs/pr/1234 (or cherry-pick or whatever you prefer.)

@Fishrock123
Copy link
Contributor

Noted, but that is kinda annoying.

cc @iojs/collaborators to notify the rest of the change.

@silverwind
Copy link
Contributor Author

Another alternative is to make curl follow redirects by default:

echo '-L' >> ~/.curlrc

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants