A collection of some things I’ve noticed related to browsing around recent commits using “git log” and how to find out if the patch was reviewed, how to find the review, etc.
In the past when doing “git log” I could find the review (and understand that a patch had been reviewed), simply by seeing a line such as
Differential Revision: ⚙ D123456 [C89/C2x] Diagnose calls to a function without a prototype but passes arguments
in the commit message. It was also easy to just open that link if I wanted to look at the code reveiw for some reason.
Nowadays, with the pull requests it seems like I should look out for a (#123456) in the first line of the commit message. Such as
[Docs] Add example of making a PR with git and GitHub web interface (#65393)
That makes it pretty easy to find out that there has been a review, and I also get the number for the pull request. But it isn’t obvious to me that the reference is to a pull request, and for example not that the author has added it as a reference to the github issue that is being solvedf. And if I want to look at the code review I need to first browse to github and then find a place where I can paste the number.
Would it be possible to configure github to add the pull-request reference as a full link at the end of the commit message instead of just a number in the commit slogan?
It is not super important to fix that, but I think I prefer how it used to be.
One ugly thing that I’ve noticed related to those pull-request references is this:
commit c39b50444cc5c9e3d1e370d1899723251e0befa1
Author: Michael Maitland michaeltmaitland@gmail.com
Date: Thu Sep 7 10:33:02 2023 -0400[RISCV][llvm-mca] Add llvm-mca tests for SiFive7 Vector Integer Arith… (#65283) …metic The intention of this test file long term is to test all valid (LMUL, SEW) pairs for each SchedWrite class. For this reason, we do not test every single instruction under every (LMUL, SEW) pair, since multiple instructions may use the same SchedWrite. For example, vadd.vv and vsub.vv both use the WriteVIALUV class.
Here it looks like the addition of (#65283) to the first line also resulted in some line wrapping of that first line, in the middle of a word.
Why? Is it possible to disable those line wraps somehow?
Another thing that is a bit confusing is that historically “PR” has been used as an abbreviation for “problem report” (not sure if that is 100% correct, but I think it at least referred to a bug report of some kind). I think I’ve still seen people talk about PR1234456 when referring to fixing a but report (even after the transition from bugzillla to github issues). But now a PR also refer to a Pull Request. That might be confusing sometimes. Specially when saying PR123456 (is it a issue of a pull request), and similarly #123456 could both mean an issue or a pull request.
Not sure what we can do about that. But it would perhaps be good to avoid both PR123456 and #123456. At least without explicictly saying “issue #123456, or pull-request #123456”.
How should we refer to bug reports and code reviews nowadays? Is a PR a bug report or a code review? Is it OK to use #123456 and in which context?
