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

51–60 of 210 posts

Re: Reorient GitHub pull requests around changesets

#51

Post by falcojr »

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…

> Honestly, squash merges are frequently a sign of people who lack mastery in Git itself or make no effort to produce high-quality independent commits.

So literally any open source project that accepts external contributions. For this reason we default to squash merge but will allow for exceptions if people ask for it and know how to structure their commits.

Re: Reorient GitHub pull requests around changesets

#52

Post by piotrkaminski »

dansiemens wrote: post #49
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.

I'm curious, are you allowed to name any folks using CodeCommit? I thought it wasn't particularly popular.

Re: Reorient GitHub pull requests around changesets

#53

Post by jayroh »

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…

Question: does CodeApprove place related files in closer proximity during review? I would _love_ to have a class and its test next to each other instead of sorted alphabetically. I'm tired of jumping around all over the place, trying to traverse through my review thought process.

Re: Reorient GitHub pull requests around changesets

#55

Post by eddythompson80 »

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.

I don’t think there is a one size fits all answer here. “Oops” commits make blame useless too plus they make following any meaningful change very hard.

You want small commits in main, so send small PRs. If you have a feature, you can do a feature branch and merge into it the small commits then merge (don’t squash) feature branch into main.

Re: Reorient GitHub pull requests around changesets

#56

Post by mbakke »

juped wrote: post #48
mbakke wrote: post #45

Earlier quoted context omitted.

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!

I didn't know about --first-parent, thanks! That gives me something to do while narrowing down the exact commit :)

Re: Reorient GitHub pull requests around changesets

#57

Post by epolanski »

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…

Average org already pays so many saas and I have to pay one for code reviews too when I'm already paying GitHub?

Also, there isn't only paying customers having this issue but large OS too.

Re: Reorient GitHub pull requests around changesets

#58

Post by mhh__ »

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…

That's like saying it's OK to expect (say) Ford to make a car with no steering wheel. GitHub basically define the baseline for the entire industry and have millions upon millions to blow on trying to do better.

(OK, Saab did actually do that once but they're weird)

Re: Reorient GitHub pull requests around changesets

#59

Post by mbakke »

eddythompson80 wrote: post #55
mbakke wrote: post #45

Earlier quoted context omitted.

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.

I don’t think there is a one size fits all answer here. “Oops” commits make blame useless too plus they make following any meaningful change very hard. You want small commits in main, so send small PRs. If you have a feature, you can do a feature branch and merge into it the small commits then merge (don’t squash) feature branch into main.

I see now that the grandparent was enforcing squash commits, did not mean to single out your comment.

It just blows my mind as someone with a somewhat perfectionist approach to rebasing (to the extent I sometimes intentionally break up or reorder commits to make the changes more intentional or natural, even when it causes nasty local merge conflicts) :P

Re: Reorient GitHub pull requests around changesets

#60

Post by allendoerfer »

piotrkaminski wrote: post #29
rat9988 wrote: post #24

Earlier quoted context omitted.

"I think in the end we should not expect GitHub to provide the best option here. We should expect them to provide a basic option (which they do) and for sophisticated consumers to pay more for a much better option. Everyone should be shopping for code review tools! " I understand this linke of thinking might suit you but I fear it is not as convincing as it sounds to you. At least it's not to me.

Here's how I like to think about it: GitHub is a generalist. They have a big platform with lots of features besides code review, so even though they also have lots of employees they won't be able to focus on code review as much as a dedicated company could. They also have a huge number of users to please so they can't afford to rock the boat too much or make the learning curve too steep. I think therefore it's pretty…

GitHub is Microsoft, which also does Excel, Azure, Windows, Teams and many other platforms, each bigger than a code review tool.
Post reply on HN