On Thu, May 2, 2024 at 8:35 AM Pedro Alves <pe...@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.
