{"thread":{"id":"55956","subject":"Only 27% of reviewed-by tags are explicit, and much more","startedAt":"2021-06-18T03:47:32Z","lastAt":"2021-06-19T21:45:46Z","messageCount":7,"participants":["Felipe Contreras","Eric Sunshine","Bagas Sanjaya"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"427806","messageId":"60cc1749b1c4d_5d12520825@natae.notmuch","threadId":"55956","inReplyTo":null,"subject":"Only 27% of reviewed-by tags are explicit, and much more","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-18T03:47:21Z","receivedAt":"2021-06-18T03:47:32Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nThis is what SubmittingPatches says about the Reviewed-by commit\nmessage trailers:\n\n `Reviewed-by:`, unlike the other tags, can only be offered by the\n  reviewer and means that she is completely satisfied that the patch\n  is ready for application.  It is usually offered only after a\n  detailed review.\n\nHowever, a quick look at the recently given Reviewed-by trailers shows\nthat isn't always the case: sometimes the patch submitter adds the\ntrailer [1], and sometimes it's the maintainer himself [2].\n\nAny generous reading of the relevant mails would conclude that the label\nwas meant to be given, albeit it was not expressly so and thus strictly\nspeaking not following the guidelines to a T:\n\n  1. 3127ff90ea (packfile-uri.txt: fix blobPackfileUri description)\n\n  This looks good - it's exactly the same as the change I approved\n  previously [3]\n\n  2. 4e689d8171 (dir: update stale description of treat_directory())\n\n  Looks good to me [4]\n\n  3. 5f03e5126d (refs: cleanup directories when deleting packed ref)\n\n  this v2 patch looks good to me [5]\n\nThese are just the last three, but a cursory look at others show a\nsimilar pattern.\n\nThis prompted me to write a script [6] to programmatically find statistics\nabout these trailers. Obviously it isn't perfect (as all software); it\ntries to avoid human fuzziness (like people pasting other patches with\nscissors [-- >8 --], or just straight put pasting the patch [^From: ]), but\neven so there are instances I manually had to skip [7].\n\nOver the past 10 years my script found 1604 commits with a Reviewed-by\nlabel, of those only 434 were expressly given.\n\nThat's merely a 27%.\n\nThis means what's written in the guidelines is not a widespread\npractice.\n\n\nNot content with this finding I went further and extracted statistics\nspecific about every reviewer.\n\nDoing so I found this interesting piece of information:\n\n  Jonathan Nieder: 67% (211/314)\n  Jeff King: 6% (15/248)\n\nJonathan Nieder is by far the person with most reviews, followed by Jeff\nKing, but they could not be more different. While Jonathan often offers\nhis Reviewed-by tag explicitly, Jeff rarely does so. That's why Jonathan\nhas 211 explicit Reviewed-bys, while Jeff only 15.\n\nYou can see the histogram of frequency of attributed Reviewed-by\ntrailers vs. percentage of explicit trailers here [8]. With the\nexception of Jonathan Nieder, almost *everyone* gets their Reviewed-by\ntrailers in an implicit way.\n\nSome heavyweights in the 20%-30% bracket are Stefan Beller (46/190),\nElijah Newren (12/47), Jonathan Tan (7/32), Ævar Arnfjörð Bjarmason\n(2/9). Other than that most reviewers are below 10% (40/673).\n\n\nSo where did this guideline come from?\n\nLooking back at history, the patch comes from Ramkumar Ramachandra, who\nwhile trying to improve something completely unrelated, stumbled upon\nthe following advice:\n\n - Again from Documentation/SubmittingPatches, I learned a while ago\n   that Reviewed-by, unlike Acked-by, can only be offered by the\n   reviewer and means that she is satisfied that the patch is ready\n   for application.\n\nDo you care to guess who gave that advice? Our top explicit\nreviewed-byer: Jonathan Nieder [10]. And this comes straight up from the\nLinux project; it's not a Git thing.\n\nSo, is it fair to say the explicit Reviewed-by trailer rule in\nDocumentation/SubmittingPatches is accurate? I would say unequivocally\n*no*. Only one person follows it religiously, everyone else seems to be\nrelying on either the reviewer or the maintainer to apply their best\njudgement.\n\nAnd that includes our second most prolific reviewer: Jeff King, who over\nthe past 10 years has only given his explicit Reviewed-by 15 times (6%\nof the time).\n\nI think it's fair to say our Documentation/SubmittingPatches needs to be\nupdated.\n\nHere are the top 20 reviewers over the past 10 years with their\ncorresponding explicit over total Reviewed-by count:\n\n  1. Jonathan Nieder: 67% (211/314)\n  2. Jeff King: 6% (15/248)\n  3. Stefan Beller: 24% (46/190)\n  4. Matthieu Moy: 9% (13/131)\n  5. Eric Sunshine: 14% (17/116)\n  6. Derrick Stolee: 6% (7/102)\n  7. Taylor Blau: 32% (27/83)\n  8. Michael Haggerty: 45% (25/55)\n  9. Elijah Newren: 25% (12/47)\n  10. Johannes Schindelin: 5% (2/35)\n  11. Jonathan Tan: 21% (7/32)\n  12. Nguyễn Thái Ngọc Duy: 20% (6/30)\n  13. Ronnie Sahlberg: 31% (5/16)\n  14. SZEDER Gábor: 0% (0/14)\n  15. Luke Diamand: 0% (0/13)\n  16. Felipe Contreras: 8% (1/12)\n  17. Johannes Sixt: 40% (4/10)\n  18. Ævar Arnfjörð Bjarmason: 22% (2/9)\n  19. Stefano Lattarini: 37% (3/8)\n  20. Torsten Bögershausen: 0% (0/7)\n\nOnly *two* reviewers are above 40%.\n\nIf that's not clear enough, I don't know what is.\n\nCheers.\n\n[1] https://lore.kernel.org/git/1d825dfdc70b8c658c4b6317310706bb6386f468.1620432501.git.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/20210511064554.59924-1-dyroneteng@gmail.com/\n[3] https://lore.kernel.org/git/20210524154354.268200-1-jonathantanmy@google.com/\n[4] https://lore.kernel.org/git/CABPp-BFudyBP8c_ROmxDEeuu5AfdoyVW8hNPYqVdFPFNofJnCQ@mail.gmail.com/\n[5] https://lore.kernel.org/git/YJnfV7op4yhyLqdg@coredump.intra.peff.net/\n[6] https://gist.github.com/felipec/f1289a48e31195ee1f36c28ca6e482db\n[7] https://lore.kernel.org/git/xmqqsgg7u3js.fsf@gitster.c.googlers.com/\n[8] https://i.imgur.com/NuvePpZ.png\n[9] https://lore.kernel.org/git/1285994263-31154-1-git-send-email-artagnon@gmail.com/\n[10] https://lore.kernel.org/git/20101001053721.GB6184@burratino/\n\n-- \nFelipe Contreras"},{"id":"427810","messageId":"CAPig+cTr2fEYN2y7FyKaspuaCpAq6wNtHgg_8RcrOmvsN5+XaA@mail.gmail.com","threadId":"55956","inReplyTo":"60cc1749b1c4d_5d12520825@natae.notmuch","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-18T05:05:51Z","receivedAt":"2021-06-18T05:06:06Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> This prompted me to write a script [6] to programmatically find statistics\n> about these trailers. Obviously it isn't perfect (as all software); it\n> tries to avoid human fuzziness (like people pasting other patches with\n> scissors [-- >8 --], or just straight put pasting the patch [^From: ]), but\n> even so there are instances I manually had to skip [7].\n>\n> Here are the top 20 reviewers over the past 10 years with their\n> corresponding explicit over total Reviewed-by count:\n>    ...\n>   5. Eric Sunshine: 14% (17/116)\n\nDoes your script check cover letters? Based upon a quick glance at it,\nit doesn't seem to.\n\nAlthough I've reviewed thousands of patches over the years, I almost\nnever give my Reviewed-by:; it is an exceedingly rare occurrence.\nHowever, when I do give it, it's almost always in response to the\ncover letter (saying \"this entire series is reviewed by <me>\"), not in\nresponse to individual patches. I've seen other reviewers do so, as\nwell. So, if your script doesn't take cover letters into account, then\nyou might want to revise it to do so in order to get a more accurate\npicture. In fact, if my memory is correct, some reviewers give their\nReviewed-by: to an entire series in response to one of the patches\nrather than to the cover letter, so perhaps you can come up with a\nheuristic to identify those cases too.\n"},{"id":"427811","messageId":"35417819-3b25-e346-436a-a2090a8f8c43@gmail.com","threadId":"55956","inReplyTo":"60cc1749b1c4d_5d12520825@natae.notmuch","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-06-18T05:56:03Z","receivedAt":"2021-06-18T05:56:38Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 18/06/21 10.47, Felipe Contreras wrote:\n> I think it's fair to say our Documentation/SubmittingPatches needs to be\n> updated.\n> \n\nWith what new procedure? After any reviews?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"427836","messageId":"60ccc3332428c_5eaa2208df@natae.notmuch","threadId":"55956","inReplyTo":"CAPig+cTr2fEYN2y7FyKaspuaCpAq6wNtHgg_8RcrOmvsN5+XaA@mail.gmail.com","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-18T16:00:51Z","receivedAt":"2021-06-18T16:00:55Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Eric Sunshine wrote:\n> On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n> > This prompted me to write a script [6] to programmatically find statistics\n> > about these trailers. Obviously it isn't perfect (as all software); it\n> > tries to avoid human fuzziness (like people pasting other patches with\n> > scissors [-- >8 --], or just straight put pasting the patch [^From: ]), but\n> > even so there are instances I manually had to skip [7].\n> >\n> > Here are the top 20 reviewers over the past 10 years with their\n> > corresponding explicit over total Reviewed-by count:\n> >    ...\n> >   5. Eric Sunshine: 14% (17/116)\n> \n> Does your script check cover letters? Based upon a quick glance at it,\n> it doesn't seem to.\n\nNot really, that's a good point.\n\n> Although I've reviewed thousands of patches over the years, I almost\n> never give my Reviewed-by:; it is an exceedingly rare occurrence.\n> However, when I do give it, it's almost always in response to the\n> cover letter (saying \"this entire series is reviewed by <me>\"), not in\n> response to individual patches. I've seen other reviewers do so, as\n> well. So, if your script doesn't take cover letters into account, then\n> you might want to revise it to do so in order to get a more accurate\n> picture.\n\nI've updated the script to consider all responses to the cover letter\nthat start with 'Re: '.\n\n> In fact, if my memory is correct, some reviewers give their\n> Reviewed-by: to an entire series in response to one of the patches\n> rather than to the cover letter, so perhaps you can come up with a\n> heuristic to identify those cases too.\n\nThat's true. Depending whether or not that's the exception or the rule it\nmight make sense to simply consider all Reviewed-by responses to apply\nto the entire series and make the heuristic match the cases where it's\nonly for a single patch.\n\n\nAnyway, with the updated script the explicit reviewed-bys are 40%, and\nhere are the stats:\n\n  1. Jonathan Nieder: 80% (254/314)\n  2. Jeff King: 7% (18/248)\n  3. Stefan Beller: 28% (54/190)\n  4. Matthieu Moy: 64% (84/131)\n  5. Eric Sunshine: 38% (45/116)\n  6. Derrick Stolee: 11% (12/102)\n  7. Taylor Blau: 46% (39/83)\n  8. Michael Haggerty: 76% (42/55)\n  9. Elijah Newren: 25% (12/47)\n  10. Johannes Schindelin: 11% (4/35)\n  11. Jonathan Tan: 28% (9/32)\n  12. Nguyễn Thái Ngọc Duy: 20% (6/30)\n  13. Ronnie Sahlberg: 100% (16/16)\n  14. SZEDER Gábor: 0% (0/14)\n  15. Luke Diamand: 7% (1/13)\n  16. Felipe Contreras: 8% (1/12)\n  17. Johannes Sixt: 40% (4/10)\n  18. Ævar Arnfjörð Bjarmason: 22% (2/9)\n  19. Stefano Lattarini: 37% (3/8)\n  20. Torsten Bögershausen: 0% (0/7)\n\nJonathan Nieder, Matthieu Moy, Michael Haggerty and Ronnie Sahlberg got\nconsiderably more percentage, but others didn't.\n\nThe histogram [1] shows an increase in the 60%-100% range, in particular\nthe 80% range (thanks to Jonathan Nieder), but there's still plenty\nbelow 50%.\n\nYou got considerably more, from 17 to 45, but still pretty far from a\n100%.\n\nI think the conclussion still stands: Reviewed-by isn't always expressly\ngiven, in fact, the majority of case it isn't.\n\nCheers.\n\n[1] https://i.imgur.com/gr6YjsZ.png\n\n-- \nFelipe Contreras"},{"id":"427837","messageId":"60ccc459c5dd8_5eaa22086e@natae.notmuch","threadId":"55956","inReplyTo":"35417819-3b25-e346-436a-a2090a8f8c43@gmail.com","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-18T16:05:45Z","receivedAt":"2021-06-18T16:05:57Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Bagas Sanjaya wrote:\n> On 18/06/21 10.47, Felipe Contreras wrote:\n> > I think it's fair to say our Documentation/SubmittingPatches needs to be\n> > updated.\n> \n> With what new procedure? After any reviews?\n\nSomething like this:\n\n. `Reviewed-by:`, means the reviewer is completely satisfied with the\n  patch after a detailed analysis. Unlike other tags, it's recommended\n  that the reviewers themselves offer it.\n\nIt is recommended, not a must.\n\n-- \nFelipe Contreras\n"},{"id":"427945","messageId":"CAPig+cRHBFodZ=ZJzpU208AWMSD-V6xH8zKZwmDPJk5eeu571w@mail.gmail.com","threadId":"55956","inReplyTo":"60ccc3332428c_5eaa2208df@natae.notmuch","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-19T07:00:40Z","receivedAt":"2021-06-19T07:00:55Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 18, 2021 at 12:00 PM Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Eric Sunshine wrote:\n> > On Thu, Jun 17, 2021 at 11:47 PM Felipe Contreras\n> > <felipe.contreras@gmail.com> wrote:\n> > > Here are the top 20 reviewers over the past 10 years with their\n> > > corresponding explicit over total Reviewed-by count:\n> > >    ...\n> > >   5. Eric Sunshine: 14% (17/116)\n> >\n> > Does your script check cover letters? Based upon a quick glance at it,\n> > it doesn't seem to.\n>\n> Not really, that's a good point.\n>\n> > Although I've reviewed thousands of patches over the years, I almost\n> > never give my Reviewed-by:; it is an exceedingly rare occurrence.\n> > However, when I do give it, it's almost always in response to the\n> > cover letter (saying \"this entire series is reviewed by <me>\"), not in\n> > response to individual patches. I've seen other reviewers do so, as\n> > well. So, if your script doesn't take cover letters into account, then\n> > you might want to revise it to do so in order to get a more accurate\n> > picture.\n>\n> I've updated the script to consider all responses to the cover letter\n> that start with 'Re: '.\n>\n> Anyway, with the updated script the explicit reviewed-bys are 40%, and\n> here are the stats:\n>\n>   5. Eric Sunshine: 38% (45/116)\n>\n> You got considerably more, from 17 to 45, but still pretty far from a\n> 100%.\n\nThe numbers produced by your script don't agree with my own\ninvestigative spelunking through my own mailbox. What I found is that,\nvia 33 emails, I've given my Reviewed-by: to 133 patches[1]. If the\n116 computed by your script is accurate, then that means that not all\nof my Reviewed-by:'s made it into the project, which is believable.\nNevertheless, according to my own mailbox -- accounting for 133\npatches -- I have almost certainly given an explicit Reviewed-by: to\nall 116 of those patches your script found, which means the script\noutput should probably be \"100% (116/116)\". This mismatch between your\nscript output and my own investigation throws doubt upon the\nscript-generated tallies for other people, as well.\n\n(Aside: I also gave another 37 explicit Reviewed-by:'s via another 7\nemails[2], however, the authors of those series ended up re-rolling\nafter my Reviewed-by:, so I did not count those in the numbers I\npresented above. There also have been several times when patch\nsubmitters added my Reviewed-by: in error[3], though I don't think\nthose ever made it into the project.)\n\n[1]: explicitly given Reviewed-by: (patch-count, message-id)\n\n10: CAPig+cSXi7Ct48gqkBVvBtOm6bMqDhPcxXeiK3ZytAitZXNT5Q@mail.gmail.com\n7: CAPig+cTt-TyOR8kc6YvBVLpf-bgFdJ+FVYA2NvG_Vvn7tMbBkQ@mail.gmail.com\n1: CAPig+cS7SEARwoBn25SsxhkvdJfDe56FyVjDGk+sJq2kXfDbjQ@mail.gmail.com\n1: CAPig+cS8hvX4GCsnfLBnQ4Q_AkUad=bw7rjVcaOqSEqcLZvx8w@mail.gmail.com\n1: CAPig+cQxvq3MzyB3e8-ZeVSdCot04=9p4L8CZRnpYbrmnR70_g@mail.gmail.com\n39: CAPig+cTxfheSPHJvC3_=jQjef0S7FiMEWCQ71ER7epfpeD_5OQ@mail.gmail.com\n1: CAPig+cSDxBFhnJSmH+WzNZBjY4O0OczazZ7EOqn1P=zgL+ec4g@mail.gmail.com\n4: CAPig+cRor4UXXZhoAkhOQfe6W+oC84YFmA-KwpLspuEh_A4Zng@mail.gmail.com\n1: CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com\n1: CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com\n5: CAPig+cQVaMOKtbUCWdZqYDO8ZUZkVcSJH14S=2xrZiDVJ59Xdg@mail.gmail.com\n6: CAPig+cQ2NRO4yaFkcGmUpY3TZcWkdg-vu6d7Fq7JgHzYSkcRgg@mail.gmail.com\n1: CAPig+cS4Bj4N8d1a29z8=f30owOec1pB=yF32ZUPmDH2Tu2kXA@mail.gmail.com\n1: CAPig+cQPbwM0+6yruK0VKKq2ujFLoCLogS7eQNN7WWgRjG5V0w@mail.gmail.com\n6: CAPig+cR9i1a7pxOxV4QU2TnoJWKn4mHHVT2tG3+uRysw=sc6qQ@mail.gmail.com\n7: CAPig+cR3diDfn893-ExKNZps=C7Z=M7DFAy-zbJzH3wKCmxVeQ@mail.gmail.com\n1: CAPig+cRimDGFDk7A2p2rKpJ2GR27_R=BJdpyPK4xeyDU1TruWQ@mail.gmail.com\n5: CAPig+cQPFzgEgdNUJSa9baUvc4BuJp8BHOLA88QJqW8N3hE8AQ@mail.gmail.com\n11: CAPig+cS8sJn9tV2kW=ASN2wTtKiK-H5oa8bThiiGfu_vMv7DoQ@mail.gmail.com\n1: 20160216050915.GA5765@flurp.local\n1: CAPig+cSxVdZN_wr3XuqDGuKn14J3B7s=S8OoH19v+AjMvcX6+Q@mail.gmail.com\n1: CAPig+cTaOTfwzodKSabdy9HFbF66RuEXwmvjZ8QuQVFMaVpA7w@mail.gmail.com\n1: CAPig+cSSdGeMuV1XLqROXvSeYfmkNc_N7E_pzfJWdDR6wfD80A@mail.gmail.com\n2: CAPig+cRySp4_uJYAwC-PY_Yh-YjBb0y1Tq=TwoD7d-Bpb_YANg@mail.gmail.com\n5: CAPig+cQtYjyj2dVFX_8CjyacRPd+dU6aMMXYGsoX9+q+zgjdZQ@mail.gmail.com\n1: CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com\n2: CAPig+cSC8RZJ-+uP=ZExVH2ZyexfQmLjzdjoBA7yuWkdYE4EGQ@mail.gmail.com\n3: CAPig+cQ-yLnjrsB1E-7=UXfGzuJHat6YtfS8EVRNP2dcjj_6TA@mail.gmail.com\n1: CAPig+cT0-ftZZyRORx-W2_Nit6XdgrpiyGS=pRjGtHoz1jW+Kg@mail.gmail.com\n1: CAPig+cQb6k8ktWR5Fz+dstfhFj5dZ+kpfzVk1Vp5piYJ3zy4rg@mail.gmail.com\n1: CAPig+cRd5+9nq3YNA6e1R_tvmBTHByOg-=KRWG1m3Fxb0e_vFQ@mail.gmail.com\n1: CAPig+cRpD0B2YRZYyJVUiM70AFcduTPOuJsuRFFKZE+bXttW1Q@mail.gmail.com\n1: CAPig+cSuBfuzL-NXYkvFoz5+jPrEUNfTqoMf-iAYyMSv3jDsqg@mail.gmail.com\n\n[2]: explicitly given Reviewed-by: but submitter ended up re-rolling\n\n6: CAPig+cRz4stVQWFD-NroVHft2xFvyZJi1ePX9T4zZ3k3=X6ZrA@mail.gmail.com\n6: CAPig+cSmb9wFCV+9PS4LYfd3hAH5s6ifRk8orVv+e2Q=h7F3Ag@mail.gmail.com\n6: CAPig+cQ4n5j4Q-WF-0cd=2+5eSAaimh3A7La+8Fe9Ox4anjtBQ@mail.gmail.com\n1: 20160327194948.GA9295@flurp.local\n12: CAPig+cTrh4u7vgQRXOT0a-5St2a6TV4qfhOMCVSetbQD0kGTrQ@mail.gmail.com\n1: CAPig+cTCRq9VCT7t8E9yjk4QcHYB2_qeBwGB_31keB4nTjkLkA@mail.gmail.com\n5: 20150519004356.GA12854@flurp.local\n\n[3]: Reviewed-by: not explicitly given\n\n11: CAPig+cRmz2C7mAzc7Z=ZStAxd3qDSmbC4sbwyLGKqvkf2yzVPA@mail.gmail.com\n1: CAPig+cQG16AhLPMeOFAw1GF81oXivFSDHvQ5B8kX20YGAT_BxQ@mail.gmail.com\n2: CAPig+cRryaafwP4gBLe_6ebWZo12HWtEC6e2CbbP6a5gVh6W4w@mail.gmail.com\n7: CAPig+cR0jG65LbopxqPpidaaNUSTRq9tboZpv0RPWyWUkSEGUw@mail.gmail.com\n1: CAPig+cR=u_ak_=J=gSAWfiNB01R7FBG+bCrx+k1HNAE0xHtwFQ@mail.gmail.com\n"},{"id":"427968","messageId":"60ce6584e36a3_d067c2084@natae.notmuch","threadId":"55956","inReplyTo":"CAPig+cRHBFodZ=ZJzpU208AWMSD-V6xH8zKZwmDPJk5eeu571w@mail.gmail.com","subject":"Re: Only 27% of reviewed-by tags are explicit, and much more","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-19T21:45:40Z","receivedAt":"2021-06-19T21:45:46Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Eric Sunshine wrote:\n> On Fri, Jun 18, 2021 at 12:00 PM Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n\n> > I've updated the script to consider all responses to the cover letter\n> > that start with 'Re: '.\n> >\n> > Anyway, with the updated script the explicit reviewed-bys are 40%, and\n> > here are the stats:\n> >\n> >   5. Eric Sunshine: 38% (45/116)\n> >\n> > You got considerably more, from 17 to 45, but still pretty far from a\n> > 100%.\n> \n> The numbers produced by your script don't agree with my own\n> investigative spelunking through my own mailbox. What I found is that,\n> via 33 emails, I've given my Reviewed-by: to 133 patches[1]. If the\n> 116 computed by your script is accurate, then that means that not all\n> of my Reviewed-by:'s made it into the project, which is believable.\n> Nevertheless, according to my own mailbox -- accounting for 133\n> patches -- I have almost certainly given an explicit Reviewed-by: to\n> all 116 of those patches your script found, which means the script\n> output should probably be \"100% (116/116)\".\n\nNo, the fact that you gave your Reviewed-by doesn't mean Junio took the\ntrailer, or even the patch.\n\nTake for example this one:\n\nhttps://lore.kernel.org/git/CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com/\n\nYou said Reviewed-by, but Junio put Helped-by in the commit message of\nd228eea514 (worktree: accept -f as short for --force for removal,\n2018-04-17).\n\nOr this one:\n\nhttps://lore.kernel.org/git/CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com/\n\nYou gave it, but Junio didn't even mention you in any of the commits of\nthis series, like 6b15595151 (t3200: unset core.logallrefupdates when\ntesting reflog creation, 2018-06-22).\n\nSame for:\n\nhttps://lore.kernel.org/git/CAPig+cQ2NRO4yaFkcGmUpY3TZcWkdg-vu6d7Fq7JgHzYSkcRgg@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cS4Bj4N8d1a29z8=f30owOec1pB=yF32ZUPmDH2Tu2kXA@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cTaOTfwzodKSabdy9HFbF66RuEXwmvjZ8QuQVFMaVpA7w@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cSC8RZJ-+uP=ZExVH2ZyexfQmLjzdjoBA7yuWkdYE4EGQ@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cQ-yLnjrsB1E-7=UXfGzuJHat6YtfS8EVRNP2dcjj_6TA@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cT0-ftZZyRORx-W2_Nit6XdgrpiyGS=pRjGtHoz1jW+Kg@mail.gmail.com/\n\nThese were never merged:\n\nhttps://lore.kernel.org/git/20160216050915.GA5765@flurp.local/\nhttps://lore.kernel.org/git/CAPig+cSSdGeMuV1XLqROXvSeYfmkNc_N7E_pzfJWdDR6wfD80A@mail.gmail.com/\n\nThis one Junio changed the title of the patch, so the script could not\nfind it:\n\nhttps://lore.kernel.org/git/CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com/\n\nHowever, there were instances where my code did skip your mails because\nthey were in a patch series that was sent as in-reply-to another patch\nseries. My code did not consider those cases.\n\nTaking a look at all the instances from all the people where a\nReviewed-by is nested deep inside a thread I noticed that with very few\nexceptions almost all of them refer to the entire patch series. So I\nupdated the code to simply consider all the replies in the thread.\n\nI ran the numbers yet again and now it's 62% explicit Reviewed-bys.\n\nYou have 94% making you the second top explicit giver, there's still 4\ncommits that got your Reviewed-by implicitly, and 2 that my script\ncould not find because the title was changed.\n\nhttps://lore.kernel.org/git/CAPig+cQxAxFUFE8j2O7iaZoAby9ioNd6Wf1OVAr5qU7kTrQOyQ@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cRgLyjNW+7EwXZB-=xNej=FR_1dqneR8VaaHzFaYHiOBA@mail.gmail.com/\nhttps://lore.kernel.org/git/CAPig+cTFsZCowqNxmNtr1za+O6KjZmqJBZLGmUFd77rHmD+5pQ@mail.gmail.com/\nhttps://lore.kernel.org/git/20150918222524.GA22410@flurp.local/\n\n1. Jonathan Nieder: 89% (281/314)\n2. Jeff King: 11% (28/248)\n3. Stefan Beller: 69% (132/190)\n4. Matthieu Moy: 67% (88/131)\n5. Eric Sunshine: 94% (110/116)\n6. Derrick Stolee: 71% (73/102)\n7. Taylor Blau: 75% (63/83)\n8. Michael Haggerty: 85% (47/55)\n9. Elijah Newren: 89% (42/47)\n10. Johannes Schindelin: 48% (17/35)\n11. Jonathan Tan: 71% (23/32)\n12. Nguyễn Thái Ngọc Duy: 33% (10/30)\n13. Ronnie Sahlberg: 100% (16/16)\n14. SZEDER Gábor: 0% (0/14)\n15. Luke Diamand: 7% (1/13)\n16. Felipe Contreras: 8% (1/12)\n17. Johannes Sixt: 40% (4/10)\n18. Ævar Arnfjörð Bjarmason: 66% (6/9)\n19. Stefano Lattarini: 62% (5/8)\n20. Torsten Bögershausen: 28% (2/7)\n\nAlthough we have many people in the 60%, 70%, 80%, and 90% range,\nthere's still plenty in 10% and 20%.\n\nHere's the histogram:\n\nhttps://i.imgur.com/jwqEp5H.png\n\nStill pretty far from 100%.\n\nCheers.\n\n-- \nFelipe Contreras"}]}