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

Reorient GitHub pull requests around changesets

mitchellh.com

Post reply on HN
Print view View original

41–50 of 210 posts

Re: Reorient GitHub pull requests around changesets

#41

Post by avita1 »

stefan_ wrote: post #36
oldtownroad wrote: post #10

I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.

So you have endless "Fix a" "Typo" "fixup" "revert redo" "add y missed in z" commits and then the squash pushes all that crap into the commit message for whatever the final mess will be? Here is a random series from DRM patchwork: https://patchwork.kernel.org/project/dri-devel/list/?series=... Would you squash that? Hell no, of course not. You would be mixing atomic changes in different subsystems. It breaks bisect a…

> So you have endless "Fix a" "Typo" "fixup" "revert redo" "add y missed in z" commits and then the squash pushes all that crap into the commit message for whatever the final mess will be?

In Github at least you can set the behavior to take the PR description by default as the squashed commit message. In fairness this is not the default. The default behavior for squash merges is to ask for a new commit message right as you hit the merge button, and the default is all of the messages from the commits being squashed together.

> make no effort to produce high-quality independent commits

I'm partial to sqaush merges when using github. I don't put much effort into the individual commit messages, instead I put lots of effort into the PR description (the thing reviewers will read, and what will eventually become the commit message in revision history). That said, one of my favorite features from gerrit at a past job was that the commit message itself could be reviewed.

Re: Reorient GitHub pull requests around changesets

#42

Post by noirscape »

codeapprove wrote: post #19

I agree 1000%. I’m the creator of what I believe is a better review interface for GitHub ( https://codeapprove.com ) but there are also many others: * CodeApprove (codeapprove.com) * Graphite (graphite.dev) * Reviewable (reviewable.io) * Axolo (axolo.co) * Viezly (viezly.com) * Mergeboard (mergeboard.com) * Codestream (codestream.com) * Pullpo (pullpo.io) * ReviewPad (reviewpad.com) * Planar (useplanar.com) * Visibly…

I disagree on this; GitHub should be building the best option here if they have the resources to do so. The fact that the review interface is as basic as it and so prone to accidentally marking the wrong comments as outdated is a major issue (one that other software like Phorge[0] already shows is possible if we're sticking to the realm of "non-mailing client reliant git servers").

Having to bolt extra features on top of GitHub to make it work properly is a shortcoming of GitHub, it shouldn't be an opportunity to build more tooling the customer has to pay for on top of it. Granted, I can see that conversation would get us nowhere given your income relies on selling people features GitHub is languishing on - you have an obvious interest in keeping that feature shitty.

[0]: Phabricator was disabled in 2022, Phorge is the new fork.

Re: Reorient GitHub pull requests around changesets

#43

Post by oldtownroad »

stefan_ wrote: post #36
oldtownroad wrote: post #10

I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.

So you have endless "Fix a" "Typo" "fixup" "revert redo" "add y missed in z" commits and then the squash pushes all that crap into the commit message for whatever the final mess will be? Here is a random series from DRM patchwork: https://patchwork.kernel.org/project/dri-devel/list/?series=... Would you squash that? Hell no, of course not. You would be mixing atomic changes in different subsystems. It breaks bisect a…

A Pull Request should represent one change to the system (whether that’s a new feature or a bug fix). A commit to the Pull Request branch only has value in the context of the Pull Request. If you want a Pull Request to contain multiple atomic changes then sure, you need to do what you’re describing… but that’s entirely optional (and straying even further from the idea of changesets).

Re: Reorient GitHub pull requests around changesets

#45

Post by mbakke »

eddythompson80 wrote: post #34
Aeolun wrote: post #25

Earlier quoted context omitted.

I really don’t see the difference between force pushing and not when you are going to squash merge anyway.

I’m assuming they mean squash merging into main once the PR is accepted. That way you have 1 commit on main that links back to 1 PR with N commits on it. Which is easier to follow on main branch rather than a million commits per PR

That makes "git bisect" and "git blame" fairly useless. Please don't do that. Better to keep "oops" commits than squashing a series of logical changes into one monster commit.

Re: Reorient GitHub pull requests around changesets

#46

Post by juped »

stefan_ wrote: post #36
oldtownroad wrote: post #10

I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.

So you have endless "Fix a" "Typo" "fixup" "revert redo" "add y missed in z" commits and then the squash pushes all that crap into the commit message for whatever the final mess will be? Here is a random series from DRM patchwork: https://patchwork.kernel.org/project/dri-devel/list/?series=... Would you squash that? Hell no, of course not. You would be mixing atomic changes in different subsystems. It breaks bisect a…

I see the problem as being that Github's bad git tooling blocks people from mastery in Git and actively discourages high-quality commits.

Re: Reorient GitHub pull requests around changesets

#48

Post by juped »

mbakke wrote: post #45
eddythompson80 wrote: post #34

Earlier quoted context omitted.

I’m assuming they mean squash merging into main once the PR is accepted. That way you have 1 commit on main that links back to 1 PR with N commits on it. Which is easier to follow on main branch rather than a million commits per PR

That makes "git bisect" and "git blame" fairly useless. Please don't do that. Better to keep "oops" commits than squashing a series of logical changes into one monster commit.

"oops" commits are bad for this too (harder to find the regression when there's unrelated broken states), but you're right that it's less bad.

Note that you can bisect --first-parent if you want the "just find the topic branch that introduced this" behavior, without taking away the ability to find the actual commit!

Re: Reorient GitHub pull requests around changesets

#49

Post by dansiemens »

codeapprove wrote: post #19

I agree 1000%. I’m the creator of what I believe is a better review interface for GitHub ( https://codeapprove.com ) but there are also many others: * CodeApprove (codeapprove.com) * Graphite (graphite.dev) * Reviewable (reviewable.io) * Axolo (axolo.co) * Viezly (viezly.com) * Mergeboard (mergeboard.com) * Codestream (codestream.com) * Pullpo (pullpo.io) * ReviewPad (reviewpad.com) * Planar (useplanar.com) * Visibly…

> We should expect them to provide a basic option (which they do) and for sophisticated consumers to pay more for a much better option

GitHub is fairly feature rich imo, _especially_ when compared to something like CodeCommit on AWS. I’ve been forced to use CodeCommit on client engagements and it’s absolutely horrid. Honestly if your tool supported CodeCommit I’d say the value proposition would skyrocket.

Re: Reorient GitHub pull requests around changesets

#50

Post by juped »

I love the amazing, advanced submission and review workflow you can see if you click anything with PATCH in it at https://public-inbox.org/git/. The best part is that it's fully integrated with Git!

It's distressing to me that Github spends so much money making Git worse. (There's some good parts of their whole product lineup, but the Git integration is supposed to be the centerpiece.)

Post reply on HN