Re: Updated Sourceware infrastructure plans

Fangrui Song
2024-05-02T23:11:52Z

On Thu, May 2, 2024 at 8:35 AM Pedro Alves <pedro@palves.net> wrote:
>
> On 2024-05-01 22:04, Simon Marchi wrote:
> > The Change-Id trailer works very well for Gerrit: once you have the hook
> > installed you basically never have to think about it again, and Gerrit
> > is able to track patch versions perfectly accurately. A while ago, I
> > asked patchwork developers if they would be open to support something
> > like that to track patches, and they said they wouldn't be against it
> > (provided it's not mandatory) [1]. But somebody would have to implement
> > it.
> >
> > Simon
> >
> > [1] https://github.com/getpatchwork/patchwork/issues/327
>
> +1000. It's mind boggling to me that people would accept Gerrit, which
> means that they'd accept Change-Id:, but then they wouldn't accept
> Change-Id: with a different system... :-)
>

Gerrit uses "Change-Id:" as stable identifiers to track patches.
https://gregoryszorc.com/blog/2020/01/07/problems-with-pull-requests-and-how-to-fix-them/
has some analysis how they are much better than the alternative smart way.

Perhaps URLs as stable identifiers will work better.
If a reader wants to find relevant discussions, they can just click
the link in many browsers, terminals, and editors.

Currently, searching for discussions about a specific commit requires
searching its title on https://inbox.sourceware.org/gcc-patches/ .
For older patches, I might even need to dig through
https://gcc.gnu.org/pipermail/gcc-patches/YYYY-MMMM/ archives.

I agree with Jeff that principal reviewers will drive improvement to
the code review process.
I am sharing two code review services LLVM has used.

---

Between 2012 and Sep 2023, LLVM had relied on its self-hosted
Phabricator instance for code review.
Fetching a patch to your local branch was as simple as `arc patch D12345`.
Similarly, creating or updating a patch involved `arc diff`.

I believe other code review services provide similar command line functionality,

---

In September 2023, LLVM transitioned to GitHub for code review.
I really dislike its code review service (however, this is a large
step forward than email based code review). From
https://maskray.me/blog/2023-09-09-reflections-on-llvm-switch-to-github-pull-requests

> On the other hand, GitHub structures the concept of pull requests around branches and enforces a branch-centric workflow. A pull request centers on the difference (commits) between the base branch and the feature branch. GitHub does not employ a stable identifier for commit tracking. If commits are rebased, reordered, or combined, GitHub can easily become confused.
>
> When you force-push a branch after a rebase, the user interface displays a line such as "force-pushed the BB branch from X to Y". Clicking the "compare" button in GitHub presents something like git diff X..Y, which includes unrelated commits. Ideally, GitHub would show the difference between the two patch files, as Phabricator does, but it only displays the difference between the two head commits. These unrelated in-between commits might be acceptable for projects with lower commit frequency but can be challenging for a project with a code frequency of 100+ commits every day.
>
> The fidelity of preserving inline comments after a force push has always been a weakness. The comments may be presented as "outdated". In the past, there was a notorious "lost inline comment" problem. Nowadays, the situation has improved, but some users still report that inline comments may occasionally become misplaced.

Thankfully, getcord/spr comes to a rescue.
User branches allow me to create/update a patch using `spr diff` like
`arc diff` for Phabricator.