Re: Commitfest Update

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?

On Fri, Jul 15, 2022 at 09:37:14PM -0500, Justin Pryzby wrote:
> 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.)

Or, to say it differently, since "reviewers" are preserved when a patch is
moved to the next CF, it comes as a surprise when by some other mechanism or
policy the field doesn't stay there. (If it's intended to be more like a
per-CF field, I think its behavior should be changed in the cfapp, to avoid
manual effort, and to avoid other people executing it differently.)

> 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 gather that your goal was to make the "reviewers" field more like "people who
are reviewing the current version of the patch", to make it easy to
find/isolate patch-versions which need to be reviewed, and hopefully accelarate
the process.

But IMO there's already no trouble finding the list of patches which need to be
reviewed - it's the long list that say "Needs Review" - which is what's
actually needed; that's not easy to do, which is why it's a long list, and no
amount of updating the annotations will help with that. I doubt many people
search for patches to review by seeking out those which have no reviewer (which
is not a short list anyway). I think they look for the most interesting
patches, or the ones that are going to be specifically useful to them.

Here's an idea: send out batch mails to people who are listed as reviewers for
patches which "Need Review". That starts to treat the reviewers field as a
functional thing rather than purely an annotation. Be sure in your message to
say "You are receiving this message because you're listed as a reviewer for a
patch which -Needs Review-". I think it's reasonable to get a message like
that 1 or 2 times per month (but not per-month-per-patch). Ideally it'd
include the list of patches specific to that reviewer, but I think it'd okay to
get an un-personalized email reminder 1x/month with a link.

BTW, one bulk update to make is for the few dozen patches that say "v15" on
them, and (excluding bugfixes) those are nearly all wrong. Since the field
isn't visible in cfbot, it's mostly ignored. The field is useful toward the
end of a release cycle to indicate patches that aren't intended for
consideration for the next release. Ideally, it'd also be used to indicate the
patches *are* being considered, but it seems like nobody does that and it ends
up being a surprise which patches are or are not committed (this seems weird
and easy to avoid but .. ). The patches which say "v15" are probably from
patches submitted during the v15 cycle, and now the version should be removed,
unless there's a reason to believe the patch is going to target v16 (like if a
committer has assigned themself).

Thanks for receiving my criticism well :)

--
Justin