On Fri, Jul 15, 2022 at 05:23:48PM -0700, Jacob Champion wrote:
> This is important stuff to discuss, for sure, but I also want to revisit
> the thing I put on pause, which is to clear out old Reviewer entries to
> make it easier for new reviewers to find work to do. If we're not using
> Reviewers as a historical record, is there any reason for me not to keep
> clearing that out?
> It undoes work that you and others have done to make the historical
> record more accurate, and I think that's understandably frustrating. But
> I thought we were trying to move away from that usage of it altogether.
I don't agree that I'm using it "for historical record". See 3499.
There's certainly some value in updating the cfapp to be "more accurate" for
some definition. By chance, I saw the "activity log".
https://commitfest.postgresql.org/activity/
Honestly, most of the changes seems to be for the worse (16 patches had the
review field nullified). Incomplete list of changes:
3609 - removed Nathan
3561 - removed Michael
3046 - removed PeterE
3396 - removed Tom
3396 - removed Robert and Bharath
2710 - removed Julien
3612 - removed Nathan (added by Greg)
3295 - removed Andrew
2573 - removed Daniel
3623 - removed Hou Zhijie
3260 - removed Fabien
3041 - removed Masahiko
2161 - removed Michael
I'm not suggesting to give the community regulars special treatment, but you
could reasonably assume that when they added themselves and then "didn't remove
themself", it was on purpose and not by omission. I think most of those people
would be surprised to learn that they're no longer considered to be reviewing
the patch.
> If someone put a lot of review into a patchset a few months ago, they
> absolutely deserve credit. But if that entry has been sitting with no
> feedback this month, why is it useful to keep that Reviewer around?
I don't know what to say to this.
Why do you think it's useful to remove annotations that people added ? (And, if
it were useful, why shouldn't that be implemented in the cfapp, which already
has all the needed information.)
Can you give an example of a patch where you sent a significant review, and
added yourself as a reviewer, but wouldn't mind if someone summarily removed
you, in batch ? It seems impolite to remove someone who is, in fact, a
reviewer.
The stated goal was to avoid the scenario that a would-be reviewer decides not
to review a patch because cfapp already shows someone else as a reviewer. I'm
sure that could happen, but I doubt it's something that happens frequently..
--
Justin