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

11–20 of 210 posts

Re: Reorient GitHub pull requests around changesets

#11

Post by nmadden »

da39a3ee wrote: post #7

> I'm sure I'm wrong about some detail about some of the points above. Someone is likely to say "he could've just done this to solve problem 5(a)" Yep, I'm going to do that! > Work-in-progress commits towards addressing review feedback become visible as soon as the branch is pushed. This forces contributors to address all feedback in a single commit, or for reviewers to deal with partially-addressed feedback. This po…

I did wonder about that when I read it. The idea of changesets and versions sounds an awful lot like branches and commits.

Re: Reorient GitHub pull requests around changesets

#12

Post by almostnormal »

da39a3ee wrote: post #7

> I'm sure I'm wrong about some detail about some of the points above. Someone is likely to say "he could've just done this to solve problem 5(a)" Yep, I'm going to do that! > Work-in-progress commits towards addressing review feedback become visible as soon as the branch is pushed. This forces contributors to address all feedback in a single commit, or for reviewers to deal with partially-addressed feedback. This po…

If an author is unable to produce readable commits on the branch of a PR the author is likely not able to produce readable change-sets either.

To review I like branches. Bad commits are not pleasant to look at, but they carry information, too, e.g., revealing the need for discussion. It's a bit like asynchronous pairing.

I don't think looking at the code on a website is sufficient as review in many cases, so I typically have a copy of the branch anyway. If any feature is needed, it is the possibiliy to comment on unchanged lines.

It probably depends on the scope. If the reviewer works on a more abstract level and can rely on authors to get the details right, maybe github is not ideal. Where the reviewer is almost as deeply involved as the author, a branch works nicely.

Re: Reorient GitHub pull requests around changesets

#13

Post by almostnormal »

dundarious wrote: post #9
da39a3ee wrote: post #5

When using changeset-based review, do you find yourself writing things like "This changeset still has the problem I drew attention to in my review of the previous changeset. Please see the comment there." ? I'm just curious how that works; I haven't used changesets much but it seems like this would be one inconvenient aspect.

Usually you can quickly click through the different changesets and see all the old comments. In my experience, it maybe needs one more click to follow the reference compared to similar comments referencing the current changeset.

Github can show a list of unresolved comments, and with a single click jump to the location in the state of the specific commit the comment was written at. No need to cycle though commits / state at each commit without comments.

Re: Reorient GitHub pull requests around changesets

#15

Post by bittermandel »

setheron wrote: post #2

Surprised he didn't refer to stack PR based systems like Gerrit for reference. I have never tried those apps that build a top GitHub. I remember Facebook's sapling has a web client as well that does stack reviews.

Doesn't he do just that?

> They're already a well-explored user experience problem in existing products like Gerrit and Phabricator.

Re: Reorient GitHub pull requests around changesets

#16

Post by tmpX7dMeXU »

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.

I’ve been contemplating pulling the trigger on this one myself. I’m pretty much sold on it. I’ve been on board with squash merging for years. Best thing I’ve ever done for our project. I eventually came to the realisation that the majority of people who were against it were so because some purist greybeard had beat it into them.

Re: Reorient GitHub pull requests around changesets

#18

Post by ninkendo »

bhouston wrote: post #14

This is a great suggestion and likely not that hard for GitHub to implement. I am surprised this isn’t updated more than it is.

GitHub seems to have no interest whatsoever in making the process of reviewing code any better or easier. It’s still essentially the same as it was 15 years ago.

They introduced batched review comments well over five years ago, and still haven’t fixed the obvious issues with it, like how if you’re the PR author and you’re replying to someone else’s comment, and hit Cmd+Enter, it starts a review of your own PR (requiring you to delete the comment and start over, fun!) rather than just replying. Glaring day-1 oversights of what should have been a simple feature, never fixed.

Re: Reorient GitHub pull requests around changesets

#19

Post by codeapprove »

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 (visibly.dev)
  * Codelantis (codelantis.com)
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!

Re: Reorient GitHub pull requests around changesets

#20

Post by Waterluvian »

ninkendo wrote: post #18
bhouston wrote: post #14

This is a great suggestion and likely not that hard for GitHub to implement. I am surprised this isn’t updated more than it is.

GitHub seems to have no interest whatsoever in making the process of reviewing code any better or easier. It’s still essentially the same as it was 15 years ago. They introduced batched review comments well over five years ago, and still haven’t fixed the obvious issues with it, like how if you’re the PR author and you’re replying to someone else’s comment, and hit Cmd+Enter, it starts a review of your own PR (requir…

I review my own PRs regularly. It’s how I talk through all my changes with meaningful inline comment threads. I prefer the behaviour you’re commenting about because I want to do those things in a “review” rather than pollute inboxes with one-off comments.

I think the main issue is that everyone has their happy paths through review, but those aren’t everyone’s happy paths. It’s why I like that having a variety of tools is possible. GitHub likely has to bias towards the beige middle ground that suits everyone well enough.

Post reply on HN