On 10/11/2018 01:44 PM, Sage Weil wrote:
On Thu, 11 Oct 2018, Gregory Farnum wrote:
On Thu, Oct 11, 2018 at 8:12 AM Sage Weil <sweil@xxxxxxxxxx> wrote:
On Thu, 11 Oct 2018, Patrick Donnelly wrote:
On Thu, Oct 11, 2018 at 7:03 AM Sage Weil <sweil@xxxxxxxxxx> wrote:
Should we enable a plugin like this?
https://github.com/probot/stale
It will let you configure a period of inactivity before a PR gets a
message like
This issue has been automatically marked as stale because it has
not had recent activity. It will be closed if no further
activity occurs. Thank you for your contributions.
and another period before the PR is closed.
We have a bazillion stale open PRs. :(
My thinking is that ideally we really want something like this so tha
thte open PRs reflect a real work queue. On the other hand, we also have
a big backlog of open PRs that have valid fixes (or, indirectly, bug
reports) that we probably need to go through and groom before
actually closing PRs.
There may also be issues in "Need Review" that need changed back to
"New". For those that use Github has the SSOT for what needs reviewed,
it could cause issues to become lost as I would think everyone
frequently filters out "Need review" when searching for issues to
fix/triage.
Unless/until we have some tool that tries to keep these in sync I think we
have to rely on catching/checking these during the normal bug scrubs. I
think the auto-closing will be a longer interval (maybe ~2 months?), much
less frequent than the bug scrubs when we should be noticing things that
are "stuck" in Needs Review?
I don't love the idea of auto-closing because a lot of those dead PRs
are (or at least *were*) on the shoulders of reviewers and core team
members, not the submitter. Maybe we could start using a
"pending-submitter" tag or something that would be auto-closed?
Yeah... I'm thinking this is as much a forcing fuction for the maintainers
as it is for the contributors. We can also start out with thresholds that
add the 'stale' flag and comment but don't close issues (yet) until we
have some time to do a scrub and catch up.
Of course I also have not seen a PR scrub in...years? So maybe it's
just that that needs to become a more regular part of the workflow I
and others do. :/ Is your expectation that every bug scrub includes a
review of PRs tagged with that component, and a check of untagged PRs?
Yeah, I think making PR scrubs a part of the regular process is a key
piece of this!
It seems like a big part of this is that there's sort of a window of
opportunity when PRs are hot and fresh and have a decent change of
getting reviewed and merged, but once a rebase is required it becomes
harder and harder to keep the momentum up. I'm not sure if a scrub
changes that, but the longer we can make that initial "fresh" window the
better off I think we'll be. One thing that might buy more time is to
avoid merging huge touch-everything PRs until the beginning of a
freeze. Things like options.cc are really irritating too because they
tend to change often and force submitters to make a trivial rebases
(which still requires them to pay attention and deal with the fallout).
Mark
