Reorient GitHub pull requests around changesets

I&#x27;m pretty sure this approach is somewhat similar to the mailing list git approach, where patches usually get submitted once ready and then changed depending on feedback as a wholly new submitted patch (as part of a broader conversation).<p>It&#x27;d be a useful thing to import without having to bring in the whole charade of using email and mailing lists (which most mail clients tend to be very unfavorable of in general nowadays) - there&#x27;s real advantage to doing this in a web interface instead.<p>UX would probably be more difficult though. The current workflow of &quot;submit branch, make PR, do changes on same branch, merge latest version through web interface&quot; is a big part of the ease of the Github UX. Doing a merge outside of that just by pulling in the right remote branches has always been a crapshoot <i>at best</i> and a pain in the ass at worst. Not helped by the fact that Github&#x27;s documentation on how to do it in git is obscure (intentional I&#x27;m sure; I know it&#x27;s possible but the docs are scattered and all of it recommend just using their gh CLI tool at this point).

I am slowly convinced that comments on PRs are like comments on blog posts or youtube videos. Ephemeral, irrelevant and ineffective. If you really want to &quot;reply&quot;, put up your own blog post or a &quot;reacts&quot; video. Same for code. unless it&#x27;s simple typo fixes or improvements, deeper fixes come from writing code samples yourself.<p>I recently commented on a juniors code, and put in about four lines of code showing how I would improve the performance- but to do so I of course had to bring up a REPL and put the code in and run it and had more or less done a PR.<p>Make it faster and easier to inject and piece of code into the PR flow (branch <i>pu</i>?), make the whole code base simple to run from any point in memory, fully testrigged, so &quot;just those highlighted lines&quot; can be run and everyone can see.<p>What we are doing with this OP approach is making it easier for a human brain to imagine what the runtime will be like. That&#x27;s the wrong approach - make it easier to have the runtime run over the chnaged lines of code and let people step through at PR time, and then make their changes alongside.<p>Or just leave a comment.<p>But we know what the rules are on comments on the internet

Github had this planned in their old roadmap... But then they deleted it...<p><a href="https:&#x2F;&#x2F;web.archive.org&#x2F;web&#x2F;20220831234107&#x2F;https:&#x2F;&#x2F;github.com&#x2F;github&#x2F;roadmap&#x2F;issues&#x2F;211" rel="nofollow noreferrer">https:&#x2F;&#x2F;web.archive.org&#x2F;web&#x2F;20220831234107&#x2F;https:&#x2F;&#x2F;github.co...</a>

Changesets seem like a UX nightmare. While I understand the motivation, the complexity of version control today is mind boggling.<p>We have a working copy, index, commits, branches, remotes, pull requests - all of these come into play when proposing even the simplest change to an open source repo today. The idea that adding yet another concept to the pile will make things better is something I can&#x27;t agree with.<p>Will it enable more capabilities? Yes. Adding features generally does that.<p>Aside from the &quot;it&#x27;s already complex enough&quot; argument, there&#x27;s also the fact that 90% of changes I&#x27;ve seen in my daily use of git don&#x27;t require this feature. This means the feature will be misunderstood, misused and often not used when actually needed.

GitHub actually stores all the data necessary to do this without changing your ref model as the article suggests — even if you force push, it never garbage collects the commits that the branch used to point to.<p>I work on a tool that includes a UI for diffing versions of a GitHub PR, but you can totally get the same thing via messing with GitHub.com URLs.

The changeset workflow is what Gerrit uses. Gerrit is great.<p>There&#x27;s a free service (GerritHub) that lets you use Gerrit for your public GitHub project. The learning curve for it is pretty steep if you&#x27;ve never used Gerrit before, though, and it does have some reliability issues. But for a certain kind of project it can really transform the code review experience.

&gt; I&#x27;m sure I&#x27;m wrong about some detail about some of the points above. Someone is likely to say &quot;he could&#x27;ve just done this to solve problem 5(a)&quot;<p>Yep, I&#x27;m going to do that!<p>&gt; 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.<p>This point isn&#x27;t really valid. It assumes that the contributor wants to push their new work addressing feedback to the remote as soon as they make each commit. That&#x27;s OK, so do I (in case my laptop disappears or whatever). But, it takes 1 second to make a git branch, so you can just push your new commits there and then merge or cherry-pick onto the PR branch when you&#x27;re ready.

It&#x27;s surprising to me that GitHub currently doesn&#x27;t attach review comments to specific commits, but only to timestamps, and I agree that it would be great to improve that.<p>But I don&#x27;t understand how a contributor can feel pressure to address all of the reviewers&#x27; comments in a single commit: They can commit as many times as they want, and only <i>push</i> when they feel it&#x27;s ready. And in the case where they may want to offer multiple different solutions for a reviewer to choose from, I like a small adaptation of a suggestion I saw by another commenter here, which is to make a PR for each option <i>off their existing PR&#x27;s branch</i>: When one of those option branches is accepted and merged, GitHub will fast-forward the original PR&#x27;s branch to include those commits, which is exactly what you would want and expect.

I don’t have experience with the outlined workflow, only with GitHub PR’s, but it feels like maybe the PR’s could too big if you have this problem?<p>I’m anticipating some push back on this, because I didn’t notice it mentioned anywhere else, even though there are a fair number of comments. So, I may need take some time to understand these other tools. But, short of that, for me personally, keeping things small and relatively easy to understand is the only way to maintain my sanity.

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

A proper review tool such as reviewable.io (which is on top of Github) addresses all points in his &quot;problems&quot; list.

Hmm, not saying Github is perfect, but I think there’s value in providing the simplest possible experience as the default.

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.

Generally, it feels like a bit of a farce that source code is very well well version controlled, but nothing else is.<p>Data isn&#x27;t well managed. Isn&#x27;t version controlled well. The pull request is just another type of data.<p>We can keep improving each applications model. But some day, imo, the general project of computing needs to take data more seriously &amp; develop general tools for managing data over time well &amp; consistently across apps. PRs would just be one example of something that would be better tracked.

IIUC, he&#x27;s referring to a workflow that Gerrit implements 1-to-1, isn&#x27;t he?

I often stack PRs to emulate the practice described by Mitchell but it&#x27;s not ideal as if you need to change an underlying PR l, you need to rebase all of the dependent PRs.

More and more I&#x27;m starting to appreciate the email based PR

It’s a slightly different methodology, but Git Patch Stack has worked well for us over the past year. The CLI is a huge help.<p><a href="https:&#x2F;&#x2F;git-ps.sh&#x2F;" rel="nofollow noreferrer">https:&#x2F;&#x2F;git-ps.sh&#x2F;</a>

I agree 1000%. I’m the creator of what I believe is a better review interface for GitHub (<a href="https:&#x2F;&#x2F;codeapprove.com" rel="nofollow noreferrer">https:&#x2F;&#x2F;codeapprove.com</a>) but there are also many others:<p><pre><code> * 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) </code></pre> 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!

This sounds like what they have in GitLab merge request flow: <a href="https:&#x2F;&#x2F;docs.gitlab.com&#x2F;ee&#x2F;user&#x2F;project&#x2F;merge_requests&#x2F;versions.html" rel="nofollow noreferrer">https:&#x2F;&#x2F;docs.gitlab.com&#x2F;ee&#x2F;user&#x2F;project&#x2F;merge_requests&#x2F;versi...</a>

A lot of people mentioning Gerrit, but Gitlab seems to support this too. It also removes approvals for the new changeset in case any were already present (by default, you can now disable this). It seems to have supported changesets forever so I&#x27;m surprised GitHub doesn&#x27;t.<p>I&#x27;ve also considered not allowing contributors to force push. Instead any changes would be pushed (possibly as fixup! or squash! commits) so all history of the merge request is easily accessible. To be rebased&#x2F;squashed later, of course.

I am personally a big fan of a patch stack style workflow.<p>I have created a tool called Git Patch Stack, <a href="https:&#x2F;&#x2F;git-ps.sh" rel="nofollow noreferrer">https:&#x2F;&#x2F;git-ps.sh</a> which makes it easier to manage a stack of patches, request review of them, re-request review of them, and many other features.<p>Checkout the the site and the documentation as it explains a lot. But if you have any questions feel free to join our Slack group and ask away.

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

I work with Gerrit in my job, and find a stack of patches to be a useful way to deal with things... but I&#x27;ve also seen that it <i>definitely</i> has a learning curve for people who&#x27;re not used to it. There&#x27;s something to be said for the GitHub pull-request &quot;just smush together all the commits on this branch&quot; model in terms of ease of understanding.<p>It&#x27;s possible that better tooling would help there, of course.<p>(A surprisingly common pain-point with Gerrit is when you&#x27;ve wound up with a semi-long-lasting stack of patches for some reason, and then you develop a branching tree of sub-patches and need to rebase them all when you make some change higher up. The answer of &quot;don&#x27;t let a stack last long enough that you need to do that&quot; has an appeal, of course.)

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.

isn&#x27;t this what Gitlab have? Instead of v1, 2, 3 you&#x27;re able to see them at the state of each git commit hash

Azure DevOps has some of this, it has &quot;pushes&quot;. You view the history of a PR per-push, so developers can commit early and often.<p>Still many of the same problems occur.<p>Fundamentally, I think the git emperor has no clothes. Managing commits is just too tedious, but squashing them causes too much pain.

It’s very weird that PR approvals remain “approved” after changes to the PR that was approved.

I&#x27;ve noticed listed problems, they surely do exist. but stepping back a little - proposed solution sounds like: &quot;lets make a brand new version control system for already existing version control system&quot;.

I’m in 100% agreement. It’s difficult handling reviews of juniors in the current model, as you have to address the fir usage first and how to avoid these type of issues ITA BeFORE getting to the review at hand.<p>Sign me up for change sets!

I haven&#x27;t used GitHub in a while but absolutely all of the author&#x27;s issues are fixed in gitlab and I have a hard time believing GitHub is that far behind.

I don&#x27;t think this is actually an improvement. It&#x27;s more complicated and most people don&#x27;t need this. Don&#x27;t let perfect be the enemy of good.

Jm2c, but by far the best place I&#x27;ve ever worked (my current client) we don&#x27;t do code reviews at all unless the author wants feedback.<p>PRs are good ways to defend your code base from bad code, and they were born in open source where you literally have no clue who the contributor is, but years of experience left me convinced that I don&#x27;t want such a system where there&#x27;s constant need to overview each other&#x27;s work.<p>I want to work with a team where there is high respect and trust. A team where I know I won&#x27;t like or love all the decisions others make, but I trust their judgement. Maybe they did indeed hack an ugly solution cheating the type system and automated controls. So what? What matters is if they have done so for good reasons (stuff was super urgent, a proper solution was just not worth the effort as the feature&#x2F;fix was really not important for the business).<p>This made development speed skyrocket and I&#x27;m no longer bound to infinite code reviews as if we were sending rockets on Mars.<p>I also want to say, code quality is high, but this stems both from working with great individuals that can be trusted and from much higher interaction speed.

100% this. It is the biggest problem I have by far with GitHub. I really hope they can follow this advice and do something closer to Gerrit.

It might be less correct from a taxonomy perspective but why not just a different branch for each change?

I truly believe that Reviewable is the best way to review code on GitHub, that&#x27;s why I left a Head of Engineering post to come work here.<p>Reviewable tracks code by revisions (aka changesets), not just the state of the git branch like GitHub, and many more improvements both to the broad strokes and to the small things.<p>Want to immediately see which PR&#x27;s you should review? Want to avoid getting pinged during the day to review PR&#x27;s you could have reviewed on your schedule? Want to see the status of a PR right when you open it? What about keeping your comments on the right line even as new changesets come in so you never have to review the PR from scratch again?<p>All handled by Reviewable to make you a better engineer: fewer interruptions, no repeated work, and generally respond to reviewers faster

That approach works fine in gerrit. I always preferred it.

Gerrit works really well. Github could adopt that tool..

it used to be worse. if you pushed changes that removed lines that had review comments on them, the review comments would simply disappear.<p>but yes, i liked perforce too.

I love the amazing, advanced submission and review workflow you can see if you click anything with PATCH in it at <a href="https:&#x2F;&#x2F;public-inbox.org&#x2F;git&#x2F;" rel="nofollow noreferrer">https:&#x2F;&#x2F;public-inbox.org&#x2F;git&#x2F;</a>. The best part is that it&#x27;s fully integrated with Git!<p>It&#x27;s distressing to me that Github spends so much money making Git worse. (There&#x27;s some good parts of their whole product lineup, but the Git integration is supposed to be the centerpiece.)

IOW, something like stackable PRs.