The times that I suggested are IMO minimally reasonable for not provoking a sense of rush and anxiety in people being poked.
If we assume that people adhere to the guideline "do not comment on a pull request unless you are willing to follow up on the edits" from here: https://github.com/druid-io/druid-io.github.io/blob/src/community/index.md#general-committer-guidelines and don't forget about the PRs they started reviewing, poking appears to be pointless. Poking and expecting the reviewer to respond "I didn't forget about it, I'll continue my review soon" is not a good use of the time of both people and IMO should be the common practice. Proposal reviews are good but the code should be reviewed thoroughly too. Despite the proposal got enough approvals, the PR with the actual code shouldn't be rushed into the codebase. On Fri, 25 Jan 2019 at 04:05, Gian Merlino <g...@apache.org> wrote: > The timelines you outlined seem quite slow. Especially "if there are enough > approvals, a PR could be merged not sooner than in two weeks since they > left the last review comment". IMO, rather than delaying patches by so > long, a better way to be courteous of a reviewer being too busy to review > in a timely manner is to seek review from some other committer that has > more time. > > In an extreme case, where the patch has issues that a reviewer feels must > be addressed before the patch is merged, but the reviewer does not have > time to hold up their end of that conversation, they can veto ( > https://www.apache.org/foundation/voting.html#Veto), accompanied by a > justification. This is a pretty blunt tool and should not be used much. But > it is there. > > I'm still optimistic that the discussion we've been having around proposals > is a good way to address a desire for reviewers to have their say, in a way > that doesn't slow down the development process so much. By starting > conversations about changes earlier, it is a way for contributors to come > together and agree on the general shape of changes before there is a bunch > of code to discuss. Hopefully that also makes code review more efficient in > terms of contributors' time, reviewers' time, and amount of calendar days > that PRs are open for. > > On Thu, Jan 24, 2019 at 3:41 AM Roman Leventov <leven...@apache.org> > wrote: > > > To foster calmness, respect, and consideration of people's busy > schedules I > > suggest the following etiquette: > > > > - When someone showed up in a PR and left some review comments, but > didn't > > explicitly approved the PR, poke them with comments like "@username do > you > > have more comments?" not sooner than in one week since they left the last > > review comment. > > - Poke people no more frequently than once per week. > > - If there are enough approvals, a PR could be merged not sooner than in > > two weeks since they left the last review comment. > > - If someone created a valuable PR but then stopped responding to review > > comments and there are conflicts or tests/CI don't pass, a PR could be > > recreated by another person not sooner than in three weeks since the > > author's last activity in the PR. Their authorship should be preserved by > > cherry-picking their commits, squashing them locally, rebasing on top of > > current upstream master, creating a new PR and choosing "Rebase and > merge" > > option when merging the new PR instead of the default "Squash and merge". > > >
