Re: Patch bumping (was Re: Feature Patch: Use completion to view parameter values)

[Lawrence Velázquez <larryv@xxxxxxx>]

> On Apr 13, 2021, at 9:35 AM, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> 
> Marlon wrote on Mon, Apr 12, 2021 at 11:18:43 +0300:
>> On 12 Apr 2021, at 00:24, Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>>> 1.  The patch has never been reviewed or discussed.
>>> 2.  The patch was reviewed and is acceptable, but was never applied.
>>> 3.  There was a discussion, but it ended without resolution.
>>> 4.  The patch was referred back to the author after review or discussion.
> ⋮
>>> I mention this mostly because I think the useful elapsed time before
>>> "bumping" might be different in each case.  In particular #4 seems
>>> like it could be left considerably longer, unless the patch is fixing
>>> a serious bug or security issue.
>> 
>> I would suggest the following minimum wait times before bumping:
>> 
>> * To remind about an unresolved patch (not yet reviewed, not yet responded to by author after review, not yet accepted/rejected/committed, etc.):
>>  * security issues: 2 days
>>  * critical bug fixes: 1 week
>>  * all other patches: 2 weeks
>> * Everything else: 1 month
> 
> I'm not sure what cases fall into "etc." and what cases fall into
> "everything else", nor what would a "critical" bugfix be.

"Everything else" seems like "anything not involving an unresolved
patch", which is out of scope as far as I'm concerned.

> How would differential delays affect your workflow?  A uniform criterion
> (such as "Thread has been dormant for >5 days") should be easier to apply.

The uniform criterion is definitely easier.  I'm open to giving
certain patch discussions more room to breathe, but at the granularity
of two classes (more urgent vs. less urgent) and not four or more.

> Regarding your taxonomy, would it be accurate to say that in cases
> A and B a submitted patch is awaiting resolution, whereas in case C it's
> generally a design question that's awaiting resolution?  In A+B the
> person in question is the patch submitter; in C the person in question
> is probably a regular developer.

That tracks with what I've seen, although in all cases there's at
least one patch in flight.  (After all, the role is "patch manager",
not "discussion manager".)

Type A also includes discussions wherein a contribution has been
accepted but not yet committed.

Many type C discussions are just a committer saying "I'm thinking
about committing this, what does everyone think?" and then waiting
on any feedback, from nitpicks to overarching design critique.

> (Aside: Note the terminology: "developer", not "committer", since in
> general, distinctions between people who do and don't have commit access
> shouldn't be made, except when it's necessary to actually invoke «git
> push».¹)

Sure, but commit access is relevant to patch discussions involving
noncommitting contributors (types A and B) because the commit step
is often the only thing holding things up.

> Regarding the magic numbers, I think one month is too long for case B
> (cf. my remarks today in workers/48526).  We don't want to bump _too_
> soon either, but I'd aim for something on the order of a week (for the
> first ping, again as per 48526).  "Once a week for patches ≥6 days old"
> achieves that, as would, say, "at least 48 weekend hours and at least
> 72 weekday hours".

This effectively retains my current policy for types A and B.  I'd
be fine with this, but we haven't heard Bart's rationale for longer
intervals.

> No comment from me on case C.

Should I continue bumping developer-only patch discussions at all?
If so, I'm inclined to let them simmer for longer -- perhaps a month
(as per workers/48516).  I feel pretty pesky basically reminding
committers to commit their own patches, but everyone forgets things
now and then.  Is it helpful or annoying?

-- 
vq