4 ms·

Is this exactly situation that https://help.github.com/articles/checking-out-pull-requests-locally https://help.github.com/articles/checking-out-pull-requests-.

Is this exactly situation that https://help.github.com/articles/checking-out-pull-requests-locally https://help.github.com/articles/checking-out-pull-requests-... documents? So basically: git fetch origin pull/ID/head:BRANCHNAME git checkout BRANCHNAME And now you can edit that pull request and/or make new pull request based on the previous one. Or just simply merge in master.

watson 12y ago
This also eliminates the use of the "Merge pull request" button which removes all the pull-request-merged-commits. I don't know about you guys, but I have not found them useful? Do you use the merge commits for anything?
- johnkeeping 12y ago
  I always use "--no-ff" when merging topic branches to master. The advantage is that you can keep individual commits on the topic branch readable and still get an overview of the changes between two releases with "git log --first-parent". You can also see the set of changes in a topic merge easily with "git log ${merge}^2..${merge}^1" whereas if you use a fast-forward merge it's not at all obvious which sets of commits are related.
- aaronharnly 12y ago
  We use the merge commits to trace back a given deploy or machine image to the pull request that caused it. So from a given deployed AMI, a Jenkins job can point to the pull request, and hence the full context of the feature or bug description, the back-and-forth of code review, etc.
jakub_g 12y ago
It's possible to configure it to have all PRs automatically fetched when you use `git fetch origin`: https://gist.github.com/piscisaureus/3342247 https://gist.github.com/piscisaureus/3342247 and then you'll have each PR available under sth like git checkout pr/123
- ntalbott 12y ago
  Cool, I'll have to try that! Does it also pull PR's that live on feature branches in forks of the root repo?
  - jakub_g 12y ago
    It fetches all the PRs. Once you open a PR for a project (even from a fork), github creates a local branch for that PR in the upstream project. Note though that doing a fetch will take some time the first time after you set up this - it will fetch all the historical PRs.
ntalbott 12y ago
OP author here... The result is the same, but I've found the `git am` workflow to be much smoother vs. mucking around with remotes. Some of that could be due to the nature of the OSS project I work on - ActiveMerchant - since just in the last month we've had 30+ unique contributors, and most of them have contributed a single change. My general recommendation is to make sure you try out the `git am` flow for a bit, but then just do what works best for you.
- peff 12y ago
  Keep in mind that `git am` is not identical to pulling the commits. You might be applying the commits on a different base, and certainly your committer field will be different (not to mention any substantive changes you make). As a result, the sha1 of the commits you create will be different than those of the original submitter. This makes life harder for the submitter, because they cannot ask a simple question: were my commits merged into the upstream repository? Because no, they weren't; but commits that are the moral equivalent were. Usually `git log --cherry-pick` can correlate the two, but not always. We do use `git am` upstream when working on git itself, for the reasons you indicate in the article (plus we like mailing-list based review). But it does come at a cost in managing the various versions of patches.
  - tytso 12y ago
    This is why it is a good idea for proposed patches to be sent to a mailing list, where other people other than the repo owner can more easily review patches, and then to have those patches tracked via patchwork. And when I accept a patch, even if I've had to edit the commit summary so it is parsable english (not all of my contributors speak english as a first language), I send a reply via e-mail saying "Thanks, applied".
  - ntalbott 12y ago
    Agreed, editing contributions does add a bit of extra work for contributors post-inclusion, especially if they've been using their own branch in production. That said, since this is a PR flow, the contributor will get an email from Github that their PR has been closed, and they can track directly from that PR to the closing commit. While I've often wished that Github had a proper mailing list per repository that could be used to discuss changes, I have to say that one awesome thing about PR's is how nice they make reviewing a patch. The diff view is just awesome, so now that I have a `git am` style flow with them it would be hard for me to switch to just passing patches around on a mailing list. That said, a mailing list would still be a welcome adjunct to the PR flow if done right.
andrewchilds 12y ago
The problem is that pull requests are one-way, read-only collaborations. If I want to make changes, I have to close the contributor's PR and open my own. With write access, I could do: git fetch origin pull/ID/head:BRANCHNAME git checkout BRANCHNAME // make changes git push origin pull/ID/head
- eLobato 12y ago
  You can always send a PR to the original sender PR branch.

watson 12y ago

This also eliminates the use of the "Merge pull request" button which removes all the pull-request-merged-commits. I don't know about you guys, but I have not found them useful? Do you use the merge commits for anything?

johnkeeping 12y ago
I always use "--no-ff" when merging topic branches to master. The advantage is that you can keep individual commits on the topic branch readable and still get an overview of the changes between two releases with "git log --first-parent". You can also see the set of changes in a topic merge easily with "git log ${merge}^2..${merge}^1" whereas if you use a fast-forward merge it's not at all obvious which sets of commits are related.
aaronharnly 12y ago
We use the merge commits to trace back a given deploy or machine image to the pull request that caused it. So from a given deployed AMI, a Jenkins job can point to the pull request, and hence the full context of the feature or bug description, the back-and-forth of code review, etc.

jakub_g 12y ago

It's possible to configure it to have all PRs automatically fetched when you use `git fetch origin`: https://gist.github.com/piscisaureus/3342247 https://gist.github.com/piscisaureus/3342247 and then you'll have each PR available under sth like git checkout pr/123

ntalbott 12y ago
Cool, I'll have to try that! Does it also pull PR's that live on feature branches in forks of the root repo?
- jakub_g 12y ago
  It fetches all the PRs. Once you open a PR for a project (even from a fork), github creates a local branch for that PR in the upstream project. Note though that doing a fetch will take some time the first time after you set up this - it will fetch all the historical PRs.

ntalbott 12y ago

OP author here... The result is the same, but I've found the `git am` workflow to be much smoother vs. mucking around with remotes. Some of that could be due to the nature of the OSS project I work on - ActiveMerchant - since just in the last month we've had 30+ unique contributors, and most of them have contributed a single change. My general recommendation is to make sure you try out the `git am` flow for a bit, but then just do what works best for you.

peff 12y ago
Keep in mind that `git am` is not identical to pulling the commits. You might be applying the commits on a different base, and certainly your committer field will be different (not to mention any substantive changes you make). As a result, the sha1 of the commits you create will be different than those of the original submitter. This makes life harder for the submitter, because they cannot ask a simple question: were my commits merged into the upstream repository? Because no, they weren't; but commits that are the moral equivalent were. Usually `git log --cherry-pick` can correlate the two, but not always. We do use `git am` upstream when working on git itself, for the reasons you indicate in the article (plus we like mailing-list based review). But it does come at a cost in managing the various versions of patches.
- tytso 12y ago
  This is why it is a good idea for proposed patches to be sent to a mailing list, where other people other than the repo owner can more easily review patches, and then to have those patches tracked via patchwork. And when I accept a patch, even if I've had to edit the commit summary so it is parsable english (not all of my contributors speak english as a first language), I send a reply via e-mail saying "Thanks, applied".
- ntalbott 12y ago
  Agreed, editing contributions does add a bit of extra work for contributors post-inclusion, especially if they've been using their own branch in production. That said, since this is a PR flow, the contributor will get an email from Github that their PR has been closed, and they can track directly from that PR to the closing commit. While I've often wished that Github had a proper mailing list per repository that could be used to discuss changes, I have to say that one awesome thing about PR's is how nice they make reviewing a patch. The diff view is just awesome, so now that I have a `git am` style flow with them it would be hard for me to switch to just passing patches around on a mailing list. That said, a mailing list would still be a welcome adjunct to the PR flow if done right.

andrewchilds 12y ago

The problem is that pull requests are one-way, read-only collaborations. If I want to make changes, I have to close the contributor's PR and open my own. With write access, I could do: git fetch origin pull/ID/head:BRANCHNAME git checkout BRANCHNAME // make changes git push origin pull/ID/head

eLobato 12y ago
You can always send a PR to the original sender PR branch.