Reorient GitHub pull requests around changesets

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.

There are two mentions of Gerrit in the article. Granted neither goes into detail, but both reference it as prior art on the topic.

Re: Reorient GitHub pull requests around changesets

#5

Post by da39a3ee » Sat, Sep 30, 2023, 9:40 PM UTC

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.

Re: Reorient GitHub pull requests around changesets

#6

Post by adm_ » Sat, Sep 30, 2023, 9:42 PM UTC

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.

I believe sapling uses https://github.com/ezyang/ghstack under the hood for stacked PR.

Re: Reorient GitHub pull requests around changesets

#7

Post by da39a3ee » Sat, Sep 30, 2023, 9:43 PM UTC

> 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 point isn'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'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're ready.

Re: Reorient GitHub pull requests around changesets

#8

Post by da39a3ee » Sat, Sep 30, 2023, 9:51 PM UTC

m3drano wrote: post #3

IIUC, he's referring to a workflow that Gerrit implements 1-to-1, isn't he?

Yes, he says so two and a half times.

Re: Reorient GitHub pull requests around changesets

#9

Post by dundarious » Sat, Sep 30, 2023, 9:51 PM UTC

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.

Re: Reorient GitHub pull requests around changesets

#10

Post by oldtownroad » Sat, Sep 30, 2023, 9:56 PM UTC

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.