Looks good. Robert Dale
On Mon, Jul 16, 2018 at 3:35 PM Stephen Mallette <spmalle...@gmail.com> wrote: > 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. > > > >>> >>>>>>> > > > >>> > > > >>> > > > > > >
