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
