[OpenWrt-Devel] improved handling of contributions [Was: Re: patches from 2018]

Tue Oct 29 08:25:27 EDT 2019

Hi,

after doing the inline comments, it looks to me like our concepts actually are not so far from each other’s.

We are both looking for a set of rules/guidelines on how to deal with old PRs. As it appears to me, the main difference is just that I prefer some manual interference, where you are looking for more automation:

> > 1. Those where the submitter left track after (initial) feedback
> > I would even choose a relatively short time span for that (e.g. one month).
> 
> so this probably means PRs with `needs changes` tag and defined inactivity,
> right?

In principle, yes. However, I just don't think it's really necessary to automate this.

I'd introduce a "stale" tag (as suggested by you, too) to GH. When looking at a PR with no submitter activity for X days, I'd write a comment like "Do you plan to respond/work on this? Otherwise will be closed ..." and assign the stale tag. The stale tag can be filtered as well as open/closed status. If there is no response on the stale tag for Y days, the PR is closed.
It's not like we have thousands of Pull Requests (situation might be answered differently for bug reports). I think doing the above manually is achievable without severe additional work. In contrast to automatic close, this gives the committer the chance to have a brief look at the PR, so he is able to account for special circumstances.
(Despite, with PRs being autoclosed, I wouldn't be surprised if we get lots of duplicate PRs instead of old ones reopened, which would also increase maintenance efforts.)

Nevertheless, I'm all in for closing stuff with no submitter feedback.

> 
> > 2. Those where there never was any feedback
> > However, I do not think it's fair to just close an old submission without
> > any developer (or others people's) feedback (category 2), just because
> > nobody is interested in it.
> 
> And whats the point to keep them lingering on the GH forever? My feeling is,
> that people are simply afraid to reject the PRs so they simply ignore them,
> but in the end, net result is the same.

Well, if the latter is true, rejecting would be the honest response, instead of keeping the submitter in misbelief that his contribution is just not getting enough attention.
I'd personally keep those contributions anyway and use a special tag for them, like you started to do with "needs reviewer" or also the "stale" tag. Again, this enables people that want a clean list to filter, but won't remove them in case someone gets interested eventually. I admit this is easier to achieve for GitHub compared to patchwork.
(Still, this is an opinion, I do not think there is right or wrong for this matter.)

> 
> > I'd see this differently if the old submissions would do any harm, but since
> > they are just lying around and making a list a little longer, it's not like
> > they pose a big problem.
> 
> If we're talking about following GH PR filter:
> 
>  is:pr is:open comments:0 updated:<2019-04-28 sort:created-asc
> 
> Then it yields following result:
> 
>  kernel: add kmod-frame-vector                   https://github.com/openwrt/openwrt/pull/1518
>  kernel: build kmod-dma-buf properly if required https://github.com/openwrt/openwrt/pull/1519
> 
> Both PRs from the same author, almost 1 year old. I believe, that if some bot
> would autoclose those (or at least marked those with `stale` label which would
> mean autoclose in next 30 days without any activity) it might signal the
> author, that there probably is more effort needed to make it merged.

Yes, but this can also be an argument for the opposite: If it's only two out of 170, why bother with a bot? Will two entries in 170 really bother when reviewing submission?

> 
> > one could provide a standardized
> > feedback that 1. patches do not apply anymore,
> > This will remove inapplicable patches from the list,
> 
> this could (and should) be automated and it's not an issue I would say.
> 
> > 2. it seems to be that interest in the subject isn't there
> 
> what could be more explicit then no activity for > 6 months?
> 
> >  and 3. that resubmission of a rebased patch is possible if the author wants
> >  that
> 
> indeed, but rebased patch (and additional work attached to that) wouldn't
> necessarily mean, that it wouldn't linger in the patchwork for the following
> year, until next pre-christmas cleanup.
> 
> > but reach out for those having invested time in an enhancement to OpenWrt
> 
> To me it seems, that it might make more sense to take a look around for
> inspiration, how it's being handled in other FOSS projects and try to improve
> current workflow.
> 
> This PW/GH/FS fragmentation still makes me crazy, but anyway just a quick
> ideas for PW/GH, we could handle the aging of the contributions more gradualy,
> like in more phases:
> 
>  1. change patch status from 'New' to 'Needs Review / ACK' after 30 days of
>     inactivity
> 
>     - on GH label=`needs reviewer`

Yes.

> 
>  2. change patch status from 'Needs Review / ACK' to 'Under Review' and assign
>     it randomly (to predefined set of volunteer commiters) after 60 days of
>     inactivity
> 
>     - on GH label=`awaiting review`

In principle, yes. However, the volunteers will need to deal with anything that comes on the desk.
I'd personally make that dependant on whether there are volunteers or not.

> 
>  3. change patch status from 'Under Review' to 'Needs Review / ACK' after 90 days
>     of inactivity

That's the same as after 30 days? (Chosen because there is no "stale" equivalent on PW?)

> 
>     - on GH label=`stale` and remove the randomly assigned reviewer
> 
>  4. change patch status from 'Needs Review / ACK' to 'Rejected' after 120 days
>     of inactivity, sending out some meaningful mail with a link to
>     exaplanation of the currently failed merging process on the wiki
> 
>     - on GH close the PR

As introduced earlier, I would require to do this manually, so at least someone has to have a brief look at the thing. This won't be that much work, and it prevents stuff being sorted out nobody really looked at.

Despite, if one automates, one of the crucial points will be how to measure "activity".

And how to deal with bug reports? In contrast to feature requests, a bug report cannot just be closed because nobody is interested in addressing it?

Best

Adrian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20191029/96c4a7b1/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel