Daniel Stone <dan...@fooishbar.org> writes: > On Wed, 13 Oct 2021 at 20:13, Jordan Justen <jordan.l.jus...@intel.com> wrote: >> Alyssa Rosenzweig <aly...@collabora.com> writes: >> > >> > Sorry, I'll make that point more emphatic. >> > >> > Upstream must do what's best for upstream without zero regard for the >> > whims of management. Doubly so for bad management. >> >> If the r-b process ever had any notice from any company's management, I >> haven't seen it. (Actually, I think most management would rather have >> the short sighted view of skipping code review to more quickly merge >> patches.) In terms of who to "track down", that is also a tenuous >> connection. > > All of the above is true but also totally irrelevant to the actual discussion. > > When R-b as a metric came up at the time of the first switch, I wrote > a really trivial Python script which used the GitLab API to scrape MR > discussions and pull 'Reviewed-by: ...' comments out and print a > leaderboard for number of reviewed MRs over the past calendar month. > Adapting that to look at approvals rather than comments would cut it > down to about 10 LoC. > > Whether it's Reviewed-by in the trailer or an approval, both are > explicitly designed to be machine readable, which means it's trivial > to turn it into a metric if you want to. Whether or not that's a good > idea is the problem of whoever wields it.
Correct. So let's not try to play whac-a-mole/manager/evil-genius. :) I think several people have already said that it's good to take the time to recognize the often overlooked and thankless job of reviewing. Potentially stripping Reviewed-by tags seems counter to that. Adding Approved-by from the web page could be helpful to simplify reviews for many merge-requests. But, retaining per-patch Reviewed-by is helpful in other cases. So hopefully we can add Approved-by *without* destroying Reviewed-by tags in the process. I would like to be able to use either in the cases where they make sense. -Jordan