Live data from Hacker News

Merge Pull Request Considered Harmful

blog.spreedly.com

61–70 of 115 posts

Re: Merge Pull Request Considered Harmful

#62

I'm not sure how I feel about someone writing something and making me the author. I guess attribution makes sense in the form of paraphrasing. But I tend to think of commits as literal quotations.

I see what you mean, and it almost seems like the "sin" isn't the additional editing, it's the subsequent rebase. It's one thing to require contributors to rebase, but it seems like another to rebase for them. Maybe I'm missing something, but based on the "History Is Written By the Victors" section of TFA I would have been surprised to see anyone other than @ntalbott at the top of https://github.com/Shopify/active_merchant/graphs/contributo...

Re: Merge Pull Request Considered Harmful

#63
post #39

Earlier quoted context omitted.

That's cuz Linus Torvalds doesn't have much of a bedside manner. I agree with ya'll on the quality commit message stuff, but that last third was all conjecture from your part, homey.

The last third is based on my extensive opinion having worked on open source software for my entire career, including being the ext4 subsystem maintainer and the e2fsprogs author and maintainer.

pwnd

Re: Merge Pull Request Considered Harmful

#65

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…

Agreed. Having a pull request go to a feature branch (or bug branch, or whatever your pull request is aiming to do) is the best way to go about it.

The issue described by the author is definitely more of a workflow problem than a technology or service problem.

Re: Merge Pull Request Considered Harmful

#67
What works best for me is to use a combination of the Pull Request button and to manually merge or rebase changes when necessary.

I used to strictly stay away from the Pull Request button, because it made my history "messy". Now I care less about that and more about the convenience (when it's appropriate).

Re: Merge Pull Request Considered Harmful

#68

I really do not see the problem with the rails commit history. Having those merge messages that point back to a pull request leaves lots of documentation about what was done and why it was done. I really do not know why people are so particular about their git log either. It shows an accurate history of the repo, not a revised cleaned up history. However being able to edit pull request before merging was a good thing…

Since the author of the article decided to invoke Linus, I thought I'd see what Linus thought about the merge vs rebase debate.

http://www.mail-archive.com/dri-devel@lists.sourceforge.net/...

Turns out Linus also agrees with drunken_thor, that merge messages are useful. Suppose that's why Linus added merges in the first place. ;)

Re: Merge Pull Request Considered Harmful

#69
post #60

Earlier quoted context omitted.

Yeah, I found the notion of a commit being "history worthy" kind of silly. If that's how it happened, then it's history! It's not a value judgement.

There's an argument that bisect is only useful if every commit is broken. If, as is often the case with my history, most commits don't compile, maybe there's an argument for squashing?

You can always `git bisect skip` when you hit a commit that doesn't build or you can't identify whether the bug of interest is present.

Re: Merge Pull Request Considered Harmful

#70
post #60

Earlier quoted context omitted.

Yeah, I found the notion of a commit being "history worthy" kind of silly. If that's how it happened, then it's history! It's not a value judgement.

There's an argument that bisect is only useful if every commit is broken. If, as is often the case with my history, most commits don't compile, maybe there's an argument for squashing?

Would you not just skip those? Skipping non-compiling code is the canonical example of "git bisect skip".

You could also have a little script that does something like:

    # Usage gbisect_prs bad good
    git bisect start
    for commit in "git log --since good --until bad"
        if not commit.message.startswith("Merge pull request #"):
            git bisect skip commit.hash
Turned into real code, obviously, but it'd tell git to ignore any commit that's not a PR merge.

You could also just include this into your bisect script if you're not doing it by hand, return 125 if it's not a commit you want to test.

Post reply on HN