git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Only 27% of reviewed-by tags are explicit, and much more
@ 2021-06-18  3:47 Felipe Contreras
  2021-06-18  5:05 ` Eric Sunshine
  2021-06-18  5:56 ` Bagas Sanjaya
  0 siblings, 2 replies; 7+ messages in thread
From: Felipe Contreras @ 2021-06-18  3:47 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff King

Hi,

This is what SubmittingPatches says about the Reviewed-by commit
message trailers:

 `Reviewed-by:`, unlike the other tags, can only be offered by the
  reviewer and means that she is completely satisfied that the patch
  is ready for application.  It is usually offered only after a
  detailed review.

However, a quick look at the recently given Reviewed-by trailers shows
that isn't always the case: sometimes the patch submitter adds the
trailer [1], and sometimes it's the maintainer himself [2].

Any generous reading of the relevant mails would conclude that the label
was meant to be given, albeit it was not expressly so and thus strictly
speaking not following the guidelines to a T:

  1. 3127ff90ea (packfile-uri.txt: fix blobPackfileUri description)

  This looks good - it's exactly the same as the change I approved
  previously [3]

  2. 4e689d8171 (dir: update stale description of treat_directory())

  Looks good to me [4]

  3. 5f03e5126d (refs: cleanup directories when deleting packed ref)

  this v2 patch looks good to me [5]

These are just the last three, but a cursory look at others show a
similar pattern.

This prompted me to write a script [6] to programmatically find statistics
about these trailers. Obviously it isn't perfect (as all software); it
tries to avoid human fuzziness (like people pasting other patches with
scissors [-- >8 --], or just straight put pasting the patch [^From: ]), but
even so there are instances I manually had to skip [7].

Over the past 10 years my script found 1604 commits with a Reviewed-by
label, of those only 434 were expressly given.

That's merely a 27%.

This means what's written in the guidelines is not a widespread
practice.


Not content with this finding I went further and extracted statistics
specific about every reviewer.

Doing so I found this interesting piece of information:

  Jonathan Nieder: 67% (211/314)
  Jeff King: 6% (15/248)

Jonathan Nieder is by far the person with most reviews, followed by Jeff
King, but they could not be more different. While Jonathan often offers
his Reviewed-by tag explicitly, Jeff rarely does so. That's why Jonathan
has 211 explicit Reviewed-bys, while Jeff only 15.

You can see the histogram of frequency of attributed Reviewed-by
trailers vs. percentage of explicit trailers here [8]. With the
exception of Jonathan Nieder, almost *everyone* gets their Reviewed-by
trailers in an implicit way.

Some heavyweights in the 20%-30% bracket are Stefan Beller (46/190),
Elijah Newren (12/47), Jonathan Tan (7/32), Ævar Arnfjörð Bjarmason
(2/9). Other than that most reviewers are below 10% (40/673).


So where did this guideline come from?

Looking back at history, the patch comes from Ramkumar Ramachandra, who
while trying to improve something completely unrelated, stumbled upon
the following advice:

 - Again from Documentation/SubmittingPatches, I learned a while ago
   that Reviewed-by, unlike Acked-by, can only be offered by the
   reviewer and means that she is satisfied that the patch is ready
   for application.

Do you care to guess who gave that advice? Our top explicit
reviewed-byer: Jonathan Nieder [10]. And this comes straight up from the
Linux project; it's not a Git thing.

So, is it fair to say the explicit Reviewed-by trailer rule in
Documentation/SubmittingPatches is accurate? I would say unequivocally
*no*. Only one person follows it religiously, everyone else seems to be
relying on either the reviewer or the maintainer to apply their best
judgement.

And that includes our second most prolific reviewer: Jeff King, who over
the past 10 years has only given his explicit Reviewed-by 15 times (6%
of the time).

I think it's fair to say our Documentation/SubmittingPatches needs to be
updated.

Here are the top 20 reviewers over the past 10 years with their
corresponding explicit over total Reviewed-by count:

  1. Jonathan Nieder: 67% (211/314)
  2. Jeff King: 6% (15/248)
  3. Stefan Beller: 24% (46/190)
  4. Matthieu Moy: 9% (13/131)
  5. Eric Sunshine: 14% (17/116)
  6. Derrick Stolee: 6% (7/102)
  7. Taylor Blau: 32% (27/83)
  8. Michael Haggerty: 45% (25/55)
  9. Elijah Newren: 25% (12/47)
  10. Johannes Schindelin: 5% (2/35)
  11. Jonathan Tan: 21% (7/32)
  12. Nguyễn Thái Ngọc Duy: 20% (6/30)
  13. Ronnie Sahlberg: 31% (5/16)
  14. SZEDER Gábor: 0% (0/14)
  15. Luke Diamand: 0% (0/13)
  16. Felipe Contreras: 8% (1/12)
  17. Johannes Sixt: 40% (4/10)
  18. Ævar Arnfjörð Bjarmason: 22% (2/9)
  19. Stefano Lattarini: 37% (3/8)
  20. Torsten Bögershausen: 0% (0/7)

Only *two* reviewers are above 40%.

If that's not clear enough, I don't know what is.

Cheers.

[1] https://lore.kernel.org/git/1d825dfdc70b8c658c4b6317310706bb6386f468.1620432501.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/20210511064554.59924-1-dyroneteng@gmail.com/
[3] https://lore.kernel.org/git/20210524154354.268200-1-jonathantanmy@google.com/
[4] https://lore.kernel.org/git/CABPp-BFudyBP8c_ROmxDEeuu5AfdoyVW8hNPYqVdFPFNofJnCQ@mail.gmail.com/
[5] https://lore.kernel.org/git/YJnfV7op4yhyLqdg@coredump.intra.peff.net/
[6] https://gist.github.com/felipec/f1289a48e31195ee1f36c28ca6e482db
[7] https://lore.kernel.org/git/xmqqsgg7u3js.fsf@gitster.c.googlers.com/
[8] https://i.imgur.com/NuvePpZ.png
[9] https://lore.kernel.org/git/1285994263-31154-1-git-send-email-artagnon@gmail.com/
[10] https://lore.kernel.org/git/20101001053721.GB6184@burratino/

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-18  3:47 Only 27% of reviewed-by tags are explicit, and much more Felipe Contreras
@ 2021-06-18  5:05 ` Eric Sunshine
  2021-06-18 16:00   ` Felipe Contreras
  2021-06-18  5:56 ` Bagas Sanjaya
  1 sibling, 1 reply; 7+ messages in thread
From: Eric Sunshine @ 2021-06-18  5:05 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Git List, Junio C Hamano, Jonathan Nieder, Jeff King

On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> This prompted me to write a script [6] to programmatically find statistics
> about these trailers. Obviously it isn't perfect (as all software); it
> tries to avoid human fuzziness (like people pasting other patches with
> scissors [-- >8 --], or just straight put pasting the patch [^From: ]), but
> even so there are instances I manually had to skip [7].
>
> Here are the top 20 reviewers over the past 10 years with their
> corresponding explicit over total Reviewed-by count:
>    ...
>   5. Eric Sunshine: 14% (17/116)

Does your script check cover letters? Based upon a quick glance at it,
it doesn't seem to.

Although I've reviewed thousands of patches over the years, I almost
never give my Reviewed-by:; it is an exceedingly rare occurrence.
However, when I do give it, it's almost always in response to the
cover letter (saying "this entire series is reviewed by <me>"), not in
response to individual patches. I've seen other reviewers do so, as
well. So, if your script doesn't take cover letters into account, then
you might want to revise it to do so in order to get a more accurate
picture. In fact, if my memory is correct, some reviewers give their
Reviewed-by: to an entire series in response to one of the patches
rather than to the cover letter, so perhaps you can come up with a
heuristic to identify those cases too.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-18  3:47 Only 27% of reviewed-by tags are explicit, and much more Felipe Contreras
  2021-06-18  5:05 ` Eric Sunshine
@ 2021-06-18  5:56 ` Bagas Sanjaya
  2021-06-18 16:05   ` Felipe Contreras
  1 sibling, 1 reply; 7+ messages in thread
From: Bagas Sanjaya @ 2021-06-18  5:56 UTC (permalink / raw)
  To: Felipe Contreras, git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff King

On 18/06/21 10.47, Felipe Contreras wrote:
> I think it's fair to say our Documentation/SubmittingPatches needs to be
> updated.
> 

With what new procedure? After any reviews?

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-18  5:05 ` Eric Sunshine
@ 2021-06-18 16:00   ` Felipe Contreras
  2021-06-19  7:00     ` Eric Sunshine
  0 siblings, 1 reply; 7+ messages in thread
From: Felipe Contreras @ 2021-06-18 16:00 UTC (permalink / raw)
  To: Eric Sunshine, Felipe Contreras
  Cc: Git List, Junio C Hamano, Jonathan Nieder, Jeff King

Eric Sunshine wrote:
> On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > This prompted me to write a script [6] to programmatically find statistics
> > about these trailers. Obviously it isn't perfect (as all software); it
> > tries to avoid human fuzziness (like people pasting other patches with
> > scissors [-- >8 --], or just straight put pasting the patch [^From: ]), but
> > even so there are instances I manually had to skip [7].
> >
> > Here are the top 20 reviewers over the past 10 years with their
> > corresponding explicit over total Reviewed-by count:
> >    ...
> >   5. Eric Sunshine: 14% (17/116)
> 
> Does your script check cover letters? Based upon a quick glance at it,
> it doesn't seem to.

Not really, that's a good point.

> Although I've reviewed thousands of patches over the years, I almost
> never give my Reviewed-by:; it is an exceedingly rare occurrence.
> However, when I do give it, it's almost always in response to the
> cover letter (saying "this entire series is reviewed by <me>"), not in
> response to individual patches. I've seen other reviewers do so, as
> well. So, if your script doesn't take cover letters into account, then
> you might want to revise it to do so in order to get a more accurate
> picture.

I've updated the script to consider all responses to the cover letter
that start with 'Re: '.

> In fact, if my memory is correct, some reviewers give their
> Reviewed-by: to an entire series in response to one of the patches
> rather than to the cover letter, so perhaps you can come up with a
> heuristic to identify those cases too.

That's true. Depending whether or not that's the exception or the rule it
might make sense to simply consider all Reviewed-by responses to apply
to the entire series and make the heuristic match the cases where it's
only for a single patch.


Anyway, with the updated script the explicit reviewed-bys are 40%, and
here are the stats:

  1. Jonathan Nieder: 80% (254/314)
  2. Jeff King: 7% (18/248)
  3. Stefan Beller: 28% (54/190)
  4. Matthieu Moy: 64% (84/131)
  5. Eric Sunshine: 38% (45/116)
  6. Derrick Stolee: 11% (12/102)
  7. Taylor Blau: 46% (39/83)
  8. Michael Haggerty: 76% (42/55)
  9. Elijah Newren: 25% (12/47)
  10. Johannes Schindelin: 11% (4/35)
  11. Jonathan Tan: 28% (9/32)
  12. Nguyễn Thái Ngọc Duy: 20% (6/30)
  13. Ronnie Sahlberg: 100% (16/16)
  14. SZEDER Gábor: 0% (0/14)
  15. Luke Diamand: 7% (1/13)
  16. Felipe Contreras: 8% (1/12)
  17. Johannes Sixt: 40% (4/10)
  18. Ævar Arnfjörð Bjarmason: 22% (2/9)
  19. Stefano Lattarini: 37% (3/8)
  20. Torsten Bögershausen: 0% (0/7)

Jonathan Nieder, Matthieu Moy, Michael Haggerty and Ronnie Sahlberg got
considerably more percentage, but others didn't.

The histogram [1] shows an increase in the 60%-100% range, in particular
the 80% range (thanks to Jonathan Nieder), but there's still plenty
below 50%.

You got considerably more, from 17 to 45, but still pretty far from a
100%.

I think the conclussion still stands: Reviewed-by isn't always expressly
given, in fact, the majority of case it isn't.

Cheers.

[1] https://i.imgur.com/gr6YjsZ.png

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-18  5:56 ` Bagas Sanjaya
@ 2021-06-18 16:05   ` Felipe Contreras
  0 siblings, 0 replies; 7+ messages in thread
From: Felipe Contreras @ 2021-06-18 16:05 UTC (permalink / raw)
  To: Bagas Sanjaya, Felipe Contreras, git
  Cc: Junio C Hamano, Jonathan Nieder, Jeff King

Bagas Sanjaya wrote:
> On 18/06/21 10.47, Felipe Contreras wrote:
> > I think it's fair to say our Documentation/SubmittingPatches needs to be
> > updated.
> 
> With what new procedure? After any reviews?

Something like this:

. `Reviewed-by:`, means the reviewer is completely satisfied with the
  patch after a detailed analysis. Unlike other tags, it's recommended
  that the reviewers themselves offer it.

It is recommended, not a must.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-18 16:00   ` Felipe Contreras
@ 2021-06-19  7:00     ` Eric Sunshine
  2021-06-19 21:45       ` Felipe Contreras
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Sunshine @ 2021-06-19  7:00 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Git List, Junio C Hamano, Jonathan Nieder, Jeff King

On Fri, Jun 18, 2021 at 12:00 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Eric Sunshine wrote:
> > On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> > > Here are the top 20 reviewers over the past 10 years with their
> > > corresponding explicit over total Reviewed-by count:
> > >    ...
> > >   5. Eric Sunshine: 14% (17/116)
> >
> > Does your script check cover letters? Based upon a quick glance at it,
> > it doesn't seem to.
>
> Not really, that's a good point.
>
> > Although I've reviewed thousands of patches over the years, I almost
> > never give my Reviewed-by:; it is an exceedingly rare occurrence.
> > However, when I do give it, it's almost always in response to the
> > cover letter (saying "this entire series is reviewed by <me>"), not in
> > response to individual patches. I've seen other reviewers do so, as
> > well. So, if your script doesn't take cover letters into account, then
> > you might want to revise it to do so in order to get a more accurate
> > picture.
>
> I've updated the script to consider all responses to the cover letter
> that start with 'Re: '.
>
> Anyway, with the updated script the explicit reviewed-bys are 40%, and
> here are the stats:
>
>   5. Eric Sunshine: 38% (45/116)
>
> You got considerably more, from 17 to 45, but still pretty far from a
> 100%.

The numbers produced by your script don't agree with my own
investigative spelunking through my own mailbox. What I found is that,
via 33 emails, I've given my Reviewed-by: to 133 patches[1]. If the
116 computed by your script is accurate, then that means that not all
of my Reviewed-by:'s made it into the project, which is believable.
Nevertheless, according to my own mailbox -- accounting for 133
patches -- I have almost certainly given an explicit Reviewed-by: to
all 116 of those patches your script found, which means the script
output should probably be "100% (116/116)". This mismatch between your
script output and my own investigation throws doubt upon the
script-generated tallies for other people, as well.

(Aside: I also gave another 37 explicit Reviewed-by:'s via another 7
emails[2], however, the authors of those series ended up re-rolling
after my Reviewed-by:, so I did not count those in the numbers I
presented above. There also have been several times when patch
submitters added my Reviewed-by: in error[3], though I don't think
those ever made it into the project.)

[1]: explicitly given Reviewed-by: (patch-count, message-id)

10: CAPig+cSXi7Ct48gqkBVvBtOm6bMqDhPcxXeiK3ZytAitZXNT5Q@mail.gmail.com
7: CAPig+cTt-TyOR8kc6YvBVLpf-bgFdJ+FVYA2NvG_Vvn7tMbBkQ@mail.gmail.com
1: CAPig+cS7SEARwoBn25SsxhkvdJfDe56FyVjDGk+sJq2kXfDbjQ@mail.gmail.com
1: CAPig+cS8hvX4GCsnfLBnQ4Q_AkUad=bw7rjVcaOqSEqcLZvx8w@mail.gmail.com
1: CAPig+cQxvq3MzyB3e8-ZeVSdCot04=9p4L8CZRnpYbrmnR70_g@mail.gmail.com
39: CAPig+cTxfheSPHJvC3_=jQjef0S7FiMEWCQ71ER7epfpeD_5OQ@mail.gmail.com
1: CAPig+cSDxBFhnJSmH+WzNZBjY4O0OczazZ7EOqn1P=zgL+ec4g@mail.gmail.com
4: CAPig+cRor4UXXZhoAkhOQfe6W+oC84YFmA-KwpLspuEh_A4Zng@mail.gmail.com
1: CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com
1: CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com
5: CAPig+cQVaMOKtbUCWdZqYDO8ZUZkVcSJH14S=2xrZiDVJ59Xdg@mail.gmail.com
6: CAPig+cQ2NRO4yaFkcGmUpY3TZcWkdg-vu6d7Fq7JgHzYSkcRgg@mail.gmail.com
1: CAPig+cS4Bj4N8d1a29z8=f30owOec1pB=yF32ZUPmDH2Tu2kXA@mail.gmail.com
1: CAPig+cQPbwM0+6yruK0VKKq2ujFLoCLogS7eQNN7WWgRjG5V0w@mail.gmail.com
6: CAPig+cR9i1a7pxOxV4QU2TnoJWKn4mHHVT2tG3+uRysw=sc6qQ@mail.gmail.com
7: CAPig+cR3diDfn893-ExKNZps=C7Z=M7DFAy-zbJzH3wKCmxVeQ@mail.gmail.com
1: CAPig+cRimDGFDk7A2p2rKpJ2GR27_R=BJdpyPK4xeyDU1TruWQ@mail.gmail.com
5: CAPig+cQPFzgEgdNUJSa9baUvc4BuJp8BHOLA88QJqW8N3hE8AQ@mail.gmail.com
11: CAPig+cS8sJn9tV2kW=ASN2wTtKiK-H5oa8bThiiGfu_vMv7DoQ@mail.gmail.com
1: 20160216050915.GA5765@flurp.local
1: CAPig+cSxVdZN_wr3XuqDGuKn14J3B7s=S8OoH19v+AjMvcX6+Q@mail.gmail.com
1: CAPig+cTaOTfwzodKSabdy9HFbF66RuEXwmvjZ8QuQVFMaVpA7w@mail.gmail.com
1: CAPig+cSSdGeMuV1XLqROXvSeYfmkNc_N7E_pzfJWdDR6wfD80A@mail.gmail.com
2: CAPig+cRySp4_uJYAwC-PY_Yh-YjBb0y1Tq=TwoD7d-Bpb_YANg@mail.gmail.com
5: CAPig+cQtYjyj2dVFX_8CjyacRPd+dU6aMMXYGsoX9+q+zgjdZQ@mail.gmail.com
1: CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com
2: CAPig+cSC8RZJ-+uP=ZExVH2ZyexfQmLjzdjoBA7yuWkdYE4EGQ@mail.gmail.com
3: CAPig+cQ-yLnjrsB1E-7=UXfGzuJHat6YtfS8EVRNP2dcjj_6TA@mail.gmail.com
1: CAPig+cT0-ftZZyRORx-W2_Nit6XdgrpiyGS=pRjGtHoz1jW+Kg@mail.gmail.com
1: CAPig+cQb6k8ktWR5Fz+dstfhFj5dZ+kpfzVk1Vp5piYJ3zy4rg@mail.gmail.com
1: CAPig+cRd5+9nq3YNA6e1R_tvmBTHByOg-=KRWG1m3Fxb0e_vFQ@mail.gmail.com
1: CAPig+cRpD0B2YRZYyJVUiM70AFcduTPOuJsuRFFKZE+bXttW1Q@mail.gmail.com
1: CAPig+cSuBfuzL-NXYkvFoz5+jPrEUNfTqoMf-iAYyMSv3jDsqg@mail.gmail.com

[2]: explicitly given Reviewed-by: but submitter ended up re-rolling

6: CAPig+cRz4stVQWFD-NroVHft2xFvyZJi1ePX9T4zZ3k3=X6ZrA@mail.gmail.com
6: CAPig+cSmb9wFCV+9PS4LYfd3hAH5s6ifRk8orVv+e2Q=h7F3Ag@mail.gmail.com
6: CAPig+cQ4n5j4Q-WF-0cd=2+5eSAaimh3A7La+8Fe9Ox4anjtBQ@mail.gmail.com
1: 20160327194948.GA9295@flurp.local
12: CAPig+cTrh4u7vgQRXOT0a-5St2a6TV4qfhOMCVSetbQD0kGTrQ@mail.gmail.com
1: CAPig+cTCRq9VCT7t8E9yjk4QcHYB2_qeBwGB_31keB4nTjkLkA@mail.gmail.com
5: 20150519004356.GA12854@flurp.local

[3]: Reviewed-by: not explicitly given

11: CAPig+cRmz2C7mAzc7Z=ZStAxd3qDSmbC4sbwyLGKqvkf2yzVPA@mail.gmail.com
1: CAPig+cQG16AhLPMeOFAw1GF81oXivFSDHvQ5B8kX20YGAT_BxQ@mail.gmail.com
2: CAPig+cRryaafwP4gBLe_6ebWZo12HWtEC6e2CbbP6a5gVh6W4w@mail.gmail.com
7: CAPig+cR0jG65LbopxqPpidaaNUSTRq9tboZpv0RPWyWUkSEGUw@mail.gmail.com
1: CAPig+cR=u_ak_=J=gSAWfiNB01R7FBG+bCrx+k1HNAE0xHtwFQ@mail.gmail.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Only 27% of reviewed-by tags are explicit, and much more
  2021-06-19  7:00     ` Eric Sunshine
@ 2021-06-19 21:45       ` Felipe Contreras
  0 siblings, 0 replies; 7+ messages in thread
From: Felipe Contreras @ 2021-06-19 21:45 UTC (permalink / raw)
  To: Eric Sunshine, Felipe Contreras
  Cc: Git List, Junio C Hamano, Jonathan Nieder, Jeff King

Eric Sunshine wrote:
> On Fri, Jun 18, 2021 at 12:00 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:

> > I've updated the script to consider all responses to the cover letter
> > that start with 'Re: '.
> >
> > Anyway, with the updated script the explicit reviewed-bys are 40%, and
> > here are the stats:
> >
> >   5. Eric Sunshine: 38% (45/116)
> >
> > You got considerably more, from 17 to 45, but still pretty far from a
> > 100%.
> 
> The numbers produced by your script don't agree with my own
> investigative spelunking through my own mailbox. What I found is that,
> via 33 emails, I've given my Reviewed-by: to 133 patches[1]. If the
> 116 computed by your script is accurate, then that means that not all
> of my Reviewed-by:'s made it into the project, which is believable.
> Nevertheless, according to my own mailbox -- accounting for 133
> patches -- I have almost certainly given an explicit Reviewed-by: to
> all 116 of those patches your script found, which means the script
> output should probably be "100% (116/116)".

No, the fact that you gave your Reviewed-by doesn't mean Junio took the
trailer, or even the patch.

Take for example this one:

https://lore.kernel.org/git/CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com/

You said Reviewed-by, but Junio put Helped-by in the commit message of
d228eea514 (worktree: accept -f as short for --force for removal,
2018-04-17).

Or this one:

https://lore.kernel.org/git/CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com/

You gave it, but Junio didn't even mention you in any of the commits of
this series, like 6b15595151 (t3200: unset core.logallrefupdates when
testing reflog creation, 2018-06-22).

Same for:

https://lore.kernel.org/git/CAPig+cQ2NRO4yaFkcGmUpY3TZcWkdg-vu6d7Fq7JgHzYSkcRgg@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cS4Bj4N8d1a29z8=f30owOec1pB=yF32ZUPmDH2Tu2kXA@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cTaOTfwzodKSabdy9HFbF66RuEXwmvjZ8QuQVFMaVpA7w@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cSC8RZJ-+uP=ZExVH2ZyexfQmLjzdjoBA7yuWkdYE4EGQ@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cQ-yLnjrsB1E-7=UXfGzuJHat6YtfS8EVRNP2dcjj_6TA@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cT0-ftZZyRORx-W2_Nit6XdgrpiyGS=pRjGtHoz1jW+Kg@mail.gmail.com/

These were never merged:

https://lore.kernel.org/git/20160216050915.GA5765@flurp.local/
https://lore.kernel.org/git/CAPig+cSSdGeMuV1XLqROXvSeYfmkNc_N7E_pzfJWdDR6wfD80A@mail.gmail.com/

This one Junio changed the title of the patch, so the script could not
find it:

https://lore.kernel.org/git/CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com/

However, there were instances where my code did skip your mails because
they were in a patch series that was sent as in-reply-to another patch
series. My code did not consider those cases.

Taking a look at all the instances from all the people where a
Reviewed-by is nested deep inside a thread I noticed that with very few
exceptions almost all of them refer to the entire patch series. So I
updated the code to simply consider all the replies in the thread.

I ran the numbers yet again and now it's 62% explicit Reviewed-bys.

You have 94% making you the second top explicit giver, there's still 4
commits that got your Reviewed-by implicitly, and 2 that my script
could not find because the title was changed.

https://lore.kernel.org/git/CAPig+cQxAxFUFE8j2O7iaZoAby9ioNd6Wf1OVAr5qU7kTrQOyQ@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cRgLyjNW+7EwXZB-=xNej=FR_1dqneR8VaaHzFaYHiOBA@mail.gmail.com/
https://lore.kernel.org/git/CAPig+cTFsZCowqNxmNtr1za+O6KjZmqJBZLGmUFd77rHmD+5pQ@mail.gmail.com/
https://lore.kernel.org/git/20150918222524.GA22410@flurp.local/

1. Jonathan Nieder: 89% (281/314)
2. Jeff King: 11% (28/248)
3. Stefan Beller: 69% (132/190)
4. Matthieu Moy: 67% (88/131)
5. Eric Sunshine: 94% (110/116)
6. Derrick Stolee: 71% (73/102)
7. Taylor Blau: 75% (63/83)
8. Michael Haggerty: 85% (47/55)
9. Elijah Newren: 89% (42/47)
10. Johannes Schindelin: 48% (17/35)
11. Jonathan Tan: 71% (23/32)
12. Nguyễn Thái Ngọc Duy: 33% (10/30)
13. Ronnie Sahlberg: 100% (16/16)
14. SZEDER Gábor: 0% (0/14)
15. Luke Diamand: 7% (1/13)
16. Felipe Contreras: 8% (1/12)
17. Johannes Sixt: 40% (4/10)
18. Ævar Arnfjörð Bjarmason: 66% (6/9)
19. Stefano Lattarini: 62% (5/8)
20. Torsten Bögershausen: 28% (2/7)

Although we have many people in the 60%, 70%, 80%, and 90% range,
there's still plenty in 10% and 20%.

Here's the histogram:

https://i.imgur.com/jwqEp5H.png

Still pretty far from 100%.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-19 21:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18  3:47 Only 27% of reviewed-by tags are explicit, and much more Felipe Contreras
2021-06-18  5:05 ` Eric Sunshine
2021-06-18 16:00   ` Felipe Contreras
2021-06-19  7:00     ` Eric Sunshine
2021-06-19 21:45       ` Felipe Contreras
2021-06-18  5:56 ` Bagas Sanjaya
2021-06-18 16:05   ` Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).