Thanks for writing https://www.bitquabit.com/post/how-i-do-github-prs/
down! This is _exactly_ how I do code reviews! Some slight
differences:

For checking stuff out locally, I use the following script:

    fn pr(&self, pr: &str, review: bool) -> anyhow::Result<()> {
        let remote = self.remote;
        let main = self.main_branch;
        cmd!(self.sh, "git fetch {remote}").run()?;
        cmd!(self.sh, "git fetch {remote} refs/pull/{pr}/head").run()?;
        cmd!(self.sh, "git switch --detach FETCH_HEAD").run()?;
        if review {
            let base = cmd!(self.sh, "git merge-base FETCH_HEAD
{remote}/{main}").read()?;
            cmd!(self.sh, "git reset {base}").run()?;
        }
        Ok(())
    }

The differences here:

* don't use `gh`, just fetch the branch directly using git. Not a big
deal, but avoids dependency on gh
* I have this `git merge-base` thing, which I _think_ works better
when the current state of the main branch is different from when the
PR branched off. Not sure if it is needed though
* I have `git reset` (`--mixed` is default anyway) as an optional flag
--- sometimes I like to check the commit history first.

Speaking of checking history --- this review flow synnergises with
magit _exceptionally_ well. This is how my review interface typically
looks:

![](https://github.com/user-attachments/assets/0bfb3ecf-5312-4121-bb2f-3670ca52be14)


Having this easily navigable diff for the entire PR is super valuable
(and you can "go to defintion" from this diff view to the actual live
code on the left).

And, yes, copying the commits manually to github is super annoying. I
am _tempted_ to try a git-only workflow, where you just go and add
review comments inline in code as a commit on top of changes, and then
just go and _push_ that commit to the shared branch. So, for the
review you iterate directly on that shared commit with the author.
Once the review is done, the top commit is reset away and the work is
merged. Hm, I wonder if I should find a like-minded colleague to try
this setup?

On Wed, Sep 11, 2024, at 05:54, Aleksey Kladov wrote:
> Thanks for writing https://www.bitquabit.com/post/how-i-do-github-prs/
> down! This is _exactly_ how I do code reviews! 

Thanks!  This was a post where I wasn't sure how much I was just writing it to my future self.  I've been happy that it seems to have resonated with some people.

> For checking stuff out locally, I use the following script:
>
>     fn pr(&self, pr: &str, review: bool) -> anyhow::Result<()> {
>         let remote = self.remote;
>         let main = self.main_branch;
>         cmd!(self.sh, "git fetch {remote}").run()?;
>         cmd!(self.sh, "git fetch {remote} refs/pull/{pr}/head").run()?;
>         cmd!(self.sh, "git switch --detach FETCH_HEAD").run()?;
>         if review {
>             let base = cmd!(self.sh, "git merge-base FETCH_HEAD
> {remote}/{main}").read()?;
>             cmd!(self.sh, "git reset {base}").run()?;
>         }
>         Ok(())
>     }

I like it!  I'm only just learning Rust, but that's a lot cleaner than I'd have expected for a Rust script.

I *think* the merge-base stuff you've got there ought to be equivalent to `git reset origin/main` instead of `git reset main`, but I'll have to double-check.  On a quick glance, they definitely look like they're equivalent from a UI perspective.

>
> Speaking of checking history --- this review flow synnergises with
> magit _exceptionally_ well
>

You don't need to sell me on magit; that alone kept me in Emacs longer than I'd have otherwise stayed, and unfortunately, I've yet to see a true equivalent outside that ecosystem.  That said, in this particular case, it's not too bad to just click on the gutter in VSC or PhpStorm and choose "stage hunk", so it's not a *huge* deal.

>
> And, yes, copying the commits manually to github is super annoying. I
> am _tempted_ to try a git-only workflow, where you just go and add
> review comments inline in code as a commit on top of changes, and then
> just go and _push_ that commit to the shared branch.
>

You may already know this, but a hedge fund called Jane Street Capital used to do this.  Their system, called Iron, did code reviews by pushing up specially-formatted comments directly to the branch.  Not only did that unify the review flow and the development flow: it also meant that *you had the full history of the review in the repository*.  This is something missing even with mailing-list-based workflows like what Sourcehut uses.  I'd definitely love to see what happens if someone brings that type of tool to Git, but I'm not volunteering for that any time soon.

--Benjamin

Wait, this is drifting off-topic, but:

On Wed, Sep 11, 2024, at 05:54, Aleksey Kladov wrote:
[snip]
> Speaking of checking history --- this review flow synnergises with
> magit _exceptionally_ well. This is how my review interface typically
> looks:
>
> ![](https://github.com/user-attachments/assets/0bfb3ecf-5312-4121-bb2f-3670ca52be14)

...are...are you running Emacs inside the Visual Studio Code terminal purely for Magit?...