I'm going to quickly summarize this thread so that hopefully by Monday we have a plan to follow.
We seem to agree that in the future we will go with the following review -the-commit (RTC) process: 1. Each change to TinkerPop release branches requires 3 +1s from committers and no -1s OR 2. A single +1 from a committer and a 1 week review period for objections (i.e. "cool down period") at which point we will assume a lazy consensus 3. The single +1 may come from the committer who submitted the PR in the first place 4. Committers are trusted with their changes but are expected to request reviews for complex changes as necessary and not rely strictly on lazy consensus 5. commit-the-review (CTR) procedures remain unchanged If there are no final comments, I will adjust our dev documentation accordingly (and then merge a bunch of PRs!) Thanks for everyone's participation, Stephen On Wed, Jul 11, 2018 at 6:50 AM Stephen Mallette <spmalle...@gmail.com> wrote: > oops - Pieter, i read your post last night then replied this morning > thinking I remembered everything you wrote - you actually called for > different email/jira lists as well.....I guess that would be helpful to > some but not others. For me personally, that would be massively burdensome > tbh. > > On Wed, Jul 11, 2018 at 6:46 AM Stephen Mallette <spmalle...@gmail.com> > wrote: > >> Thanks for everyone's thoughts - some replies, first to Jason: >> >> > but I agree that nagging is not a great path forward. >> >> Robert Dale has already expressed his sadness that my nags are going away >> >> > It'd be great to have these examples added to the maintainer >> guidelines. >> >> i've said as much before in different places but it's not in the dev >> docs. of course, depending on how this thread ends the dev docs will need >> some changes in this area all around so we'll see about adding such things. >> >> > If the contribution is a major feature or significant change, the >> expectation is that the committer realizes this and holds it open for 3 >> votes before committing. >> >> So, here's my thinking on "major feature" - if the committer had done >> things properly the "major feature" would have already had major >> visibility. There would have been some form of DISCUSS thread ahead of >> time, plus a JIRA ticket with more information. If it was a "major" third >> party submission, a committer would not +1 and walk away but suggest to the >> third-party that we need to bring this to the attention of dev in some way. >> How about this, perhaps in the DISCUSS/JiRA anyone might call for a "review >> of 3" (reserving of course for a -1 veto) in which case the "cooling >> period" wouldn't apply and we'd need the 3 +1s from committers. >> >> As for Pieter: >> >> > Perhaps for version 4 the project should be broken up >> >> Maybe, though I've come to to really like our giant uber project. Having >> everything together has so many great benefits and somehow we've managed to >> keep the whole thing relatively easy to build and deploy. Breaking up the >> project repositories however would not allow you to really hide from the >> expansiveness we've grown in to. This dev list would still be the location >> for all discussion and we'd still likely have a single JIRA instance so I >> don't see that providing much relief if you're feeling that way >> unfortunately. >> >> >> >> On Tue, Jul 10, 2018 at 4:19 PM pieter gmail <pieter.mar...@gmail.com> >> wrote: >> >>> Hi, >>> >>> I feel like the project has become a bit too big and dispersed. A large >>> portion of the emails, jira or otherwise are irrelevant to my >>> interest/time/work. >>> >>> Perhaps for version 4, TinkerPop could be broken up into more focused >>> projects with their own jira/email/process management. >>> >>> gremlin-language >>> gremlin-server >>> js-driver >>> python-driver >>> java-driver >>> .net-driver >>> reference implementation >>> ... >>> >>> Thanks >>> Pieter >>> >>> >>> >>> >>> Perhaps for version 4 the project should be broken up >>> >>> On 10/07/2018 22:01, Jason Plurad wrote: >>> > Thanks for starting this conversation, Stephen. Lots of interesting >>> tidbits >>> > here, and perhaps some we can apply to other OSS projects. >>> > >>> >> I'm not sure if committers/PMC members have just not had time to do >>> > reviews or have not felt comfortable doing them >>> > >>> > Probably a combination of both, especially with the GLVs. >>> > >>> >> I personally chase votes in the background to get PRs to >>> merge.....and, I >>> > don't want to do that anymore. >>> > >>> > Amazing that you did that, but I agree that nagging is not a great path >>> > forward. >>> > >>> >> it is perfectly fine to review/VOTE in the following manner (as >>> examples) >>> > It'd be great to have these examples added to the maintainer >>> guidelines. >>> > When I do code reviews, sometimes I feel like one-liner votes are a >>> bit of >>> > a cop out, but having examples like this would lower the mental hurdle >>> to >>> > getting started on reviewing. >>> > >>> >> It would also be nice for non-committers to do reviews - i don't know >>> how >>> > to encourage that behavior though. >>> > >>> > I agree on this, and it would be particularly important on areas of the >>> > code where we only have one primary committer, like each GLV. If we >>> come to >>> > agreement on a new policy, I'd suggest that if we get the docs written >>> up >>> > and published, then we can mention it on gremlin-users sort of as a >>> heads >>> > up to those interested in getting more involved. Their participation >>> helps >>> > drive out releases, and new releases attract more users. >>> > >>> > Regarding the proposal, a single binding +1 from a committer with a 1 >>> week >>> > lazy consensus sounds fine to me. If the contribution is a major >>> feature or >>> > significant change, the expectation is that the committer realizes >>> this and >>> > holds it open for 3 votes before committing. >>> > >>> > >>> > >>> > On Tue, Jul 10, 2018 at 1:46 PM, Stephen Mallette < >>> spmalle...@gmail.com> >>> > wrote: >>> > >>> >> 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. >>> >>>>>>> >>> >>>
