On Thu, Oct 11, 2018 at 12:33 PM Mark Nelson <mnelson@xxxxxxxxxx> wrote: > > > > 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. That's definitely not feasible, since those touch-everything PRs 1) then need to be continuously rebased, generally in much more complicated ways than when merged the other way around, 2) often take a lot of testing that needs more time to shake out. > 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). Hmm, this really shouldn't happen often unless you're futzing with the same config options as other people are. Which granted I expect may be something you see more often than most people when somebody writes a RADOS feature for you with a new config option that logically sits next to the ones you were experimenting with. :p -Greg
