Some Observations on the D Development Process

[H. S. Teoh]

On Thu, Jan 04, 2018 at 06:50:25PM +0000, Dmitry Olshansky via Digitalmars-d wrote:
> On Thursday, 4 January 2018 at 18:09:32 UTC, Seb wrote:
[...]
> > BTW: For those who haven't seen this, dlang-bot now detects these
> > stalled PRs and for now labels them as "stalled":
> > 
> > https://github.com/dlang-bots/dlang-bot/blob/master/source/dlangbot/cron.d#L60
> > 
> > I'm not sure whether auto-closing them is a good idea, but we
> > definitely need to do sth. about them.
> > Also one year without any activity is a very long time.
> 
> It’s not like we loose anything, we don’t delete code of the author.
> So should he/she find time - just create a new PR. Also these willing
> to dig in the closed PRs are free to do so (it’s just a couple of
> passes to add a filter to your search).

I used to think that it's OK to leave old PRs there, since they don't
actively cause harm.  But lately I'm changing my stance.  Having a PR
queue that's overly long conveys the wrong image to a potential
contributor.  Having PRs that are years old with no resolution is
discouraging to look at, and also a source of frustration to the
submitter, esp. when the change is not especially complicated.

So nowadays I'm inclined to suggest that if a PR hasn't had any response
from the submitter for, say, 2-3 months, it should be closed with a
comment to resubmit if the submitter still feels strongly about it. Note
that I'm measuring this by activity *from the submitter*; I don't think
it's a good idea to close a PR if the submitter is waiting for somebody
from the team to respond.  That would send the completely wrong message.

So I'd suggest that if a PR has had no activity from the submitter for,
say, 2 weeks, then send a ping. At the 1 month mark, send another ping.
At the 2 month mark, close it with a note to reopen if the submitter
later decides to work on it again.

If a PR has had no activity from the team for 1 week, then the onus is
upon us to do something about it.  I'm not sure what to do when the
reason is that none of the active team members feel qualified to handle
the proposed change.  For example, if the PR touches some numerical
algorithms, I would hesitate to review it, because of lack of expertise
in the area.  I guess this is where the lack of consistent talent is a
big liability: if the go-to person for a particular area isn't active,
or only rarely active, or just too busy, then there's really not much we
can reasonably do about it.  Perhaps one small thing that might help is
to acknowledge the PR, or take initiative to actively ping the go-to
person until he responds.  At the very least, this would send the right
message that the team cares about the contribution, but just doesn't
have the resources right now to deal with it yet.

Alternatively, and this is just a wild idea off the top of my head, if
the go-to person doesn't respond by X days (where X is some fixed number
we agree on), a non-expert team member can do a code-level review (i.e.,
check for obvious glaring problems, check for basic code sanity) and
then merge the PR.  Worse comes to worst, the revert button's always
there.  This has to be done with care, though.  Definitely should not be
done prior to an imminent release, so that we don't introduce problems
that are hard to fix later; but perhaps early in the release cycle it
might be OK, under the assumption that any obvious problem would
manifest itself sometime before the next release, and the PR can be
reverted if necessary.  But even then, we might need to mark certain
types of changes as off-limits, e.g., if they would be too disruptive,
or fundamentally changes the way things work with unknown consequences.
Ultimately, though, this is just a crutch in the face of lack of the
right talents to do a proper review.  Anyway, this is just a rough idea
that may hopefully lead to a better one.


T

-- 
"Computer Science is no more about computers than astronomy is about telescopes." -- E.W. Dijkstra