Updated dev docs (but didn't publish yet) - https://github.com/apache/tinkerpop/commit/e1d57d6e076efbc68bbdf0fe0c121054a9d5152f
On Fri, Jul 13, 2018 at 10:52 AM Ted Wilmes <twil...@gmail.com> wrote: > That looks good, thanks Stephen. > > --Ted > > On Thu, Jul 12, 2018 at 5:23 PM Stephen Mallette <spmalle...@gmail.com> > wrote: > > > 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. > > >>> >>>>>>> > > >>> > > >>> > > >
