Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Merge Pull Request Considered Harmful

blog.spreedly.com

Post reply on HN
Print view View original

1–10 of 115 posts

Re: Merge Pull Request Considered Harmful

#2

Post by shadowmint »

Hm... this seems like a very complicated way of saying that github should have a way to merge pull requests into a new branch.

...but it doesn't so you have to:

    - checkout a local copy
    - add a remote to the PR
    - checkout a new branch
    - merge the PR into your local branch
    - fix code, merge to master
Which is entirely true; it is annoying.

The simple solution, though, is to require pull requests to come in a feature branch, and flat out reject any that target master. /shrug

Re: Merge Pull Request Considered Harmful

#5

Post by MaikuMori »

Is this exactly situation that 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.

Re: Merge Pull Request Considered Harmful

#6

Post by watson »

I just tried this on an open pull request we had. Pretty ok experience. For some reason though, there where a merge conflict when running the "hub am -3 " command. In my case was easy to fix, but Github reported the PR to be mergeable, so maybe Github uses a different merge strategy for PR's than "hub am" out of the box?

Re: Merge Pull Request Considered Harmful

#7

Post by dr4g0n »

shadowmint wrote: post #2

Hm... this seems like a very complicated way of saying that github should have a way to merge pull requests into a new branch. ...but it doesn't so you have to: - checkout a local copy - add a remote to the PR - checkout a new branch - merge the PR into your local branch - fix code, merge to master Which is entirely true; it is annoying. The simple solution, though, is to require pull requests to come in a feature br…

You can create a new branch containing the pull request fairly easily.[1]

It's just: git fetch origin pull/ID/head:BRANCHNAME git checkout BRANCHNAME

Edit: I see that MaikuMori[2] posted the same information just before me; ah well.

[1]: https://help.github.com/articles/checking-out-pull-requests-...

[2]: https://news.ycombinator.com/item?id=7949107

Re: Merge Pull Request Considered Harmful

#8

Post by ytjohn »

watson wrote: post #4

Just a note to people who want to try this: According to the post you should use the command "git am -3 ", this seems to be a typo and should be: "hub am -3 "

Author didn't mention it, but in the hub documents they instruct you to `alias git=hub`. So when author is running git, he's really running hub.

Re: Merge Pull Request Considered Harmful

#9

Post by watson »

MaikuMori wrote: post #5

Is this exactly situation that 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.

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?

Re: Merge Pull Request Considered Harmful

#10

Post by davedx »

watson wrote: post #6

I just tried this on an open pull request we had. Pretty ok experience. For some reason though, there where a merge conflict when running the "hub am -3 " command. In my case was easy to fix, but Github reported the PR to be mergeable, so maybe Github uses a different merge strategy for PR's than "hub am" out of the box?

Yeah, I don't get this hub thing. When I want to manually merge on a Github project I just follow the GH instructions on the PR page to manually checkout the fork, do my work there, and merge it locally then push. Why do you need another tool to do this?
Post reply on HN