Good point, Ted - that wasn't clear and in truth I didn't think that through well. I think we could say that that the +1 would come from a committer. If the committer and submitter are one in the same then it has its single VOTE and technically, the PR just goes through the week long cooling period and could be merged after that. In the event the PR submitter is not a committer then they would require at least one committer to be on board with a +1 and a week long wait.
Ideally, I think we can trust committers enough to do smart things with this change in process. I would hope that a committer who submits a PR that is especially complex and touches scary code parts or breaks user/provider APIs requests an actual review from another committer who might be able to spot some problems even if the 1 week cool down passes by. I don't want to subvert the good things that reviews do, but I don't want them holding up simple code changes either. I'd really like it if we introduced this change and we still got multiple +1s on PRs. It would also be nice for non-committers to do reviews - i don't know how to encourage that behavior though. On Tue, Jul 10, 2018 at 1:26 PM Ted Wilmes <twil...@gmail.com> wrote: > I fell way off the PR review train, I'll get back on. For clarification, is > that a +1 on top of the submitter +1? I'm thinking you > all just meant the submitter's +1 would be adequate after the lazy > consensus period but wanted to be sure. I'd be fine to moving with that. My > impression is that with the folks involved, if a submitter feels that > another set of eyes is really required and lazy consensus is not adequate, > regardless of the policy, that review will be sought and performed prior to > merge. > > --Ted > > On Tue, Jul 10, 2018 at 11:44 AM Stephen Mallette <spmalle...@gmail.com> > wrote: > > > > It looks like its disabled for this project. > > > > I don't think we can use the GitHub integration without getting off our > > Apache Mirror (which we've discussed, but not really pulled the trigger > on > > for no particular reason other than the hassle of changing everything). > > > > > Does it have to be in that order? > > > > I was thinking that the as long as there is a single +1 at any time in > that > > week (or after that week) then it would be good to merge > > > > On Tue, Jul 10, 2018 at 12:36 PM Robert Dale <robd...@gmail.com> wrote: > > > > > There might be a better alternative to privately nagging ;-) Github > has > > a > > > feature on the sidebar that can be used to request reviews from > > individuals > > > or groups. The heading has 'Reviewers' and, when it's active, has a > gear > > > icon to select people. Github will then email the reviewers with the > > > request. It looks like its disabled for this project. > > > > > > I like the idea of adding the option of having a single vote and a week > > to > > > soak. Does it have to be in that order? Or can the vote take place > any > > > time during that week? Otherwise, you could end up with no one voting > in > > > the first week, get a vote, then waiting another week. > > > > > > Robert Dale > > > > > > > > > On Tue, Jul 10, 2018 at 7:22 AM Jorge Bay Gondra < > > jorgebaygon...@gmail.com > > > > > > > wrote: > > > > > > > I'm +1 on the idea of switching to lazy consensus after a single > > binding > > > > plus one and a week for objection. TinkerPop has so many different > > > modules > > > > / areas and committers have different expertise that is hard to get 3 > > > votes > > > > on something. > > > > > > > > Other projects have the concept of main "reviewer" and this would be > > very > > > > similar, a committer that was responsible for reviewing the code and > to > > > > check that everything is in order. > > > > > > > > El mar., 10 jul. 2018 a las 13:01, Stephen Mallette (< > > > spmalle...@gmail.com > > > > >) > > > > escribió: > > > > > > > > > I believe that the review process is not working so well anymore. > I'm > > > not > > > > > sure if committers/PMC members have just not had time to do reviews > > or > > > > have > > > > > not felt comfortable doing them, but for the most part they aren't > > > > getting > > > > > done and PRs are languishing. Personally, I like our process, but > if > > it > > > > > takes 3+ weeks to deal with a PR like this: > > > > > > > > > > https://github.com/apache/tinkerpop/pull/879/files > > > > > > > > > > where all we did was remove deprecated methods, I don't think we're > > > ever > > > > > going to get anything else through. As it stands, I personally > chase > > > > votes > > > > > in the background to get PRs to merge.....and, I don't want to do > > that > > > > > anymore. > > > > > > > > > > I'll remind committers (and those interested in becoming > committers) > > > > that a > > > > > "review" in our context doesn't have to be a full on review of code > > > where > > > > > you hold this deep understanding of everything that is going on. > That > > > is > > > > > awesome when that happens, but it is perfectly fine to review/VOTE > in > > > the > > > > > following manner (as examples): > > > > > > > > > > + VOTE +1 - ran docker integration tests and everything passes > > > > > + VOTE +1 - reviewed the code in detail - solid pull request > > > > > + VOTE +1 - agree with the principle of this pull request but don't > > > fully > > > > > understand the code > > > > > + VOTE +1 - read through the updated documentation and understand > why > > > > this > > > > > is important, nice > > > > > > > > > > So basically, you can VOTE and just explain your position for why > you > > > > voted > > > > > (or not explain). I would like to keep this process, however, if we > > > can't > > > > > raise the VOTEs for whatever reason, then I'd like to suggest a > > change. > > > > > > > > > > I'd suggest that we go to a slightly looser version of > > > > review-then-commit, > > > > > where we require the 3 binding +1 VOTEs as we have been doing OR > we > > > > > require a single binding +1 and 1 week for objection at which point > > we > > > > have > > > > > a form of lazy consensus. > > > > > > > > > > Honestly, I'd like to see some discussion on this from > committers/PMC > > > > > members and not go with the standard form of lazy consensus that we > > > > > typically end up with. However, if no one truly has anything to > say, > > > > > consider the 72 hours started now. > > > > > > > > > > > > > > >
