On Mon, Apr 1, 2019 at 9:14 AM Sönke Liebau <[email protected]> wrote:
> Thanks Kenn, > > that perfectly explains it! > Something I just thought of while reading your mail: in the special case > where the committer also considers himself an SME one review would be > sufficient I guess? > Yes, I think that is right. I think it already fits with what you said before. Trust the committer to decide. Kenn > > Best, > Sönke > > On Mon, Apr 1, 2019 at 6:09 PM Kenneth Knowles <[email protected]> wrote: > > > Great summary! One thing I didn't really understand. > > > > On Mon, Apr 1, 2019 at 8:07 AM Sönke Liebau > > <[email protected]> wrote: > > > > > For pull requests that change the content of training material the > usual > > > rules don't apply, as we probably won't have experts for all fields in > > the > > > team from the get-go. Not sure how best to handle this, should we > > initially > > > try going for simply 2 lgtm without any requirements around commiter > > > status? > > > > > > > As a practical matter you'll need to have a committer somewhere since > only > > they have permission to merge. And the committer's role then might be as > an > > expert in the organization of the training repo and the policies of the > > project. If neither the author nor the reviewer are committers, then they > > may not really know best practices that the project is trying to > establish. > > > > So this scenario is non-committer author, non-committer subject matter > > reviewer, and a committer comes in to make sure the change is basically > OK > > (not breaking anything, using the right directories, good commit message, > > etc). > > > > Kenn > > > > > > > For every commit there has to be either a jira or a pull request (both > is > > > fine too). > > > > > > I think that covers the basics, if everybody is happy with that > > > fundamentally, I'd suggest voting on it at this level and I'll then > > draft a > > > contributor guide based on that, which we can refine. > > > > > > Best regards, > > > Sönke > > > > > > On Fri, Mar 15, 2019 at 6:33 PM Dmitriy Pavlov <[email protected]> > > wrote: > > > > > > > Hi Kenneth, > > > > > > > > Thank you for sharing, it is a very good topic. I've copy-pasted it > at > > > > least to Ignite > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-PeerReviewandLGTM > > > > and > > > > I agree with this approach for Training(incubating). > > > > > > > > Sincerely, > > > > Dmitriy Pavlov > > > > > > > > пт, 15 мар. 2019 г. в 20:02, Kenneth Knowles <[email protected]>: > > > > > > > > > Here it is: > > > > > > > > > > > > > > > > > > > > https://beam.apache.org/contribute/committer-guide/#always-get-to-lgtm-looks-good-to-me > > > > > > > > > > But it should also be in the beginner contributor guide, that they > > can > > > > > review code as a way to get started. Of course, unsolicited > feedback > > is > > > > not > > > > > always welcome, so a code author needs a way to connect with them > and > > > > > request their review. > > > > > > > > > > Kenn > > > > > > > > > > On Fri, Mar 15, 2019 at 12:22 AM Lars Francke < > > [email protected]> > > > > > wrote: > > > > > > > > > > > Oh, that's a good idea. I remember the discussion, you're doing > > lots > > > of > > > > > > good things in the Beam project. That's a good way to get new > > > > > contributors > > > > > > on board and in the watchlist for new committers. > > > > > > > > > > > > As far as I can tell this is not written down in your > Contributors > > > > guide. > > > > > > Do you have it somewhere else or is this "informal"? > > > > > > > > > > > > On Fri, Mar 15, 2019 at 5:02 AM Kenneth Knowles <[email protected] > > > > > > wrote: > > > > > > > > > > > > > Interesting idea about the 72 hour lazy consensus for > committing. > > > > > Vaguely > > > > > > > related is that you can commit without review if the build is > > > broken, > > > > > > since > > > > > > > it is emergency. But you are still required to open a pull > > request > > > > and > > > > > > > merge your own pull request, so everyone can see and comment on > > it > > > > > later. > > > > > > > > > > > > > > Also, a thing that works really well for Beam is RTC where if > the > > > > > author > > > > > > is > > > > > > > a committer, the reviewer can be a non-committer. You could > also > > > have > > > > > > lazy > > > > > > > consensus after 72 hours for minor changes, but might not need > it > > > if > > > > > you > > > > > > > can trust a committer to choose a good enough non-committer > > > reviewer. > > > > > > > > > > > > > > Kenn > > > > > > > > > > > > > > On Wed, Mar 13, 2019 at 10:36 AM Craig Russell < > > > [email protected] > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Lars, > > > > > > > > > > > > > > > > I think we might codify this as RTC with an exception that if > > > > someone > > > > > > > > wants to commit a trivial or non-controversial patch, they > > still > > > > need > > > > > > to > > > > > > > > post the patch for review and specifically request lazy > > consensus > > > > > after > > > > > > > 72 > > > > > > > > hours or some other time we deem relevant to this project. > > > > > > > > > > > > > > > > Craig > > > > > > > > > > > > > > > > > On Mar 13, 2019, at 2:21 AM, Lars Francke < > > > > [email protected]> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Mar 11, 2019 at 4:34 PM Craig Russell < > > > > > [email protected] > > > > > > > > <mailto:[email protected]>> wrote: > > > > > > > > > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >>> On Mar 11, 2019, at 6:35 AM, Lars Francke < > > > > > [email protected]> > > > > > > > > >> wrote: > > > > > > > > >>> > > > > > > > > >>> Okay, any other opinions? > > > > > > > > >>> > > > > > > > > >>> I don't necessarily need "Bylaws" but a good > "Contributors > > > > Guide" > > > > > > is > > > > > > > > one > > > > > > > > >> of > > > > > > > > >>> my pet peeves. So as soon as we have the website up and > > > running > > > > > I'd > > > > > > > > like > > > > > > > > >> to > > > > > > > > >>> get some content up there. > > > > > > > > >> > > > > > > > > >> Yes, a contributors' guide is a great thing to add to the > > > > > > user-facing > > > > > > > > site. > > > > > > > > >>> > > > > > > > > >>> I'm usually in favor of review-then-commit but in our > case > > > I'm > > > > > not > > > > > > > sure > > > > > > > > >> if > > > > > > > > >>> that's always going to work. > > > > > > > > >>> Ideally we'd have people contribute stuff that neither of > > us > > > > is - > > > > > > > > >>> technically I mean - able to review as we won't have > > experts > > > > for > > > > > > > every > > > > > > > > >>> single field in the committership initially. > > > > > > > > >>> But we can of course review for form and style etc. > > > > > > > > >>> > > > > > > > > >>> Other than that I'm in favor of requiring a +1 from > another > > > > > > committer > > > > > > > > to > > > > > > > > >>> commit anything but this can be painful for small things > > > (e.g. > > > > a > > > > > > > single > > > > > > > > >>> typo). For Hive we established a rule that for small > > changes > > > > (at > > > > > > > > >>> contributors discretion) after a delay/request for > reviews > > of > > > > 72h > > > > > > or > > > > > > > so > > > > > > > > >> it > > > > > > > > >>> can be committed without feedback. > > > > > > > > >> > > > > > > > > >> This sounds reasonable. It's a combination of RTC and CTR > > with > > > > > lazy > > > > > > > > >> consensus. Formally, I think it would be described as CTR > > > with > > > > > > > > discretion > > > > > > > > >> to wait until another committer has reviewed a "large" > > change. > > > > > > > > >> > > > > > > > > > > > > > > > > > > I understand what you mean but I (others might disagree) > > would > > > > like > > > > > > to > > > > > > > > see > > > > > > > > > it codified as RTC and have the commit without a review as > > the > > > > > > > exception > > > > > > > > > not the other way around. This has been a major pain for me > > as > > > a > > > > > > "lone" > > > > > > > > > non-big-corporate contributor to various projects. You see > > > things > > > > > > > > happening > > > > > > > > > without discussion on any public mailing list/Jira. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >> Regards, > > > > > > > > >> > > > > > > > > >> Craig > > > > > > > > >>> > > > > > > > > >>> Cheers, > > > > > > > > >>> Lars > > > > > > > > >>> > > > > > > > > >>> On Tue, Feb 26, 2019 at 10:11 PM Sharan Foga < > > > > [email protected]> > > > > > > > > wrote: > > > > > > > > >>> > > > > > > > > >>>> Hi > > > > > > > > >>>> > > > > > > > > >>>> I'd be happy with having both models of Jira and Github > in > > > > place > > > > > > > too. > > > > > > > > >>>> > > > > > > > > >>>> Thanks > > > > > > > > >>>> Sharan > > > > > > > > >>>> > > > > > > > > >>>> On 2019/02/25 19:06:28, Lars Francke < > > > [email protected]> > > > > > > > wrote: > > > > > > > > >>>>> On Mon, Feb 25, 2019 at 5:37 AM Kenneth Knowles < > > > > > [email protected] > > > > > > > > > > > > > > > >> wrote: > > > > > > > > >>>>> > > > > > > > > >>>>>> On Fri, Feb 22, 2019 at 2:31 PM Lars Francke < > > > > > > > > [email protected]> > > > > > > > > >>>>>> wrote: > > > > > > > > >>>>>> > > > > > > > > >>>>>>>> > > > > > > > > >>>>>>>> Bylaws have gone out of fashion and it’s generally > > > > recommend > > > > > > > that > > > > > > > > >>>>>>> podlings > > > > > > > > >>>>>>>> (and TLP) don’t have them and use the “default”. > > > > > > > > >>>>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> Really? I didn't notice. > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>>> As the default is often unclear I've created a > default > > > > > simple > > > > > > > set > > > > > > > > >>>> that > > > > > > > > >>>>>>>> graduating projects can use, [1] Legal and board > have > > > > > reviewed > > > > > > > > >>>> this. > > > > > > > > >>>>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> That's a good starting point. What I struggle with is > > > that > > > > > > every > > > > > > > > >>>> project > > > > > > > > >>>>>>> works slightly different in things like: > > > Commit-then-review > > > > > or > > > > > > > > >>>>>>> review-then-commit or whether ReviewBoard needs to be > > > used > > > > or > > > > > > > > patches > > > > > > > > >>>>>> need > > > > > > > > >>>>>>> to be attached to Jira etc. > > > > > > > > >>>>>>> These rules are often not written down which makes it > > > hard > > > > to > > > > > > > > >>>> contribute. > > > > > > > > >>>>>>> This might be a particular pet-peeve of mine because > I > > > do - > > > > > > > nature > > > > > > > > >>>> of the > > > > > > > > >>>>>>> job - lots of "drive by" contributions but I'd still > > > love a > > > > > > > clearly > > > > > > > > >>>>>> defined > > > > > > > > >>>>>>> set of rules on how we want to operate. > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> This includes - and I don't actually know what works > > > there > > > > > > these > > > > > > > > >>>> days and > > > > > > > > >>>>>>> what doesn't - the Github use. > > > > > > > > >>>>>>> If anyone knows what's allowed and possible it'd be > > great > > > > if > > > > > > you > > > > > > > > >>>> could > > > > > > > > >>>>>>> share. > > > > > > > > >>>>>>> > > > > > > > > >>>>>> > > > > > > > > >>>>>> At this point, the GitHub pull request model is a de > > facto > > > > > > > standard > > > > > > > > in > > > > > > > > >>>> my > > > > > > > > >>>>>> mind. It works great with ASF infra. > > > > > > > > >>>>>> > > > > > > > > >>>>>> The most important thing being that you don't have to > > > read a > > > > > > > > project's > > > > > > > > >>>>>> contribution guide to know how to *request* that they > > > *pull* > > > > > > from > > > > > > > > your > > > > > > > > >>>> fork > > > > > > > > >>>>>> of their code. Great for drive-by contributions. If a > > > > project > > > > > > > > doesn't > > > > > > > > >>>>>> basically follow this model, I don't trust them to > > accept > > > > > > outside > > > > > > > > >>>> input. We > > > > > > > > >>>>>> should definitely use it. I would guess users will be > > much > > > > > more > > > > > > > > likely > > > > > > > > >>>> to > > > > > > > > >>>>>> also want to casually contribute bits here and there, > > > > compared > > > > > > to > > > > > > > > >> other > > > > > > > > >>>>>> projects. Let's make it easy and fun for them (and > bring > > > > them > > > > > on > > > > > > > > board > > > > > > > > >>>> :-). > > > > > > > > >>>>>> > > > > > > > > >>>>> > > > > > > > > >>>>> I agree that lots of projects use it but most projects > I > > > work > > > > > > with > > > > > > > > work > > > > > > > > >>>>> differently (e.g. Hadoop, Kafka, HBase etc.). Some do > > have > > > a > > > > > > model > > > > > > > > >> where > > > > > > > > >>>>> both ways are accepted (Jira & Github). I myself am not > > the > > > > > > biggest > > > > > > > > fan > > > > > > > > >>>> of > > > > > > > > >>>>> Pull requests. I'm against using it as the only > option. I > > > > like > > > > > > the > > > > > > > > >>>>> "old-fashioned" way of attaching patches to Jira. It > > > doesn't > > > > > lock > > > > > > > me > > > > > > > > >>>> into a > > > > > > > > >>>>> workflow provided by a 3rd party. I can download > > everything > > > > > > offline > > > > > > > > and > > > > > > > > >>>>> prepare a review offline as well. That's not possible > > with > > > > > Github > > > > > > > (I > > > > > > > > >> can > > > > > > > > >>>>> download the code but I can't prepare a review for the > > > > website > > > > > > > > >> offline). > > > > > > > > >>>>> > > > > > > > > >>>>> But as I said: I definitely agree that it makes > > "drive-by" > > > > > > > > >> contributions > > > > > > > > >>>>> easier so I'm in favor of having both models. > > > > > > > > >>>>> > > > > > > > > >>>>> Lars > > > > > > > > >>>>> > > > > > > > > >>>>> > > > > > > > > >>>>>> > > > > > > > > >>>>>> Kenn > > > > > > > > >>>>>> > > > > > > > > >>>>> > > > > > > > > >>>>> > > > > > > > > >>>>>> > > > > > > > > >>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>> > > > > > > > > >>>>>>>> Thanks, > > > > > > > > >>>>>>>> Justin > > > > > > > > >>>>>>>> > > > > > > > > >>>>>>>> 1. > > > > > https://wiki.apache.org/incubator/DefaultProjectGuidelines > > > > > > > > >>>>>>> > > > > > > > > >>>>>> > > > > > > > > >>>>> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > >> Craig L Russell > > > > > > > > >> Secretary, Apache Software Foundation > > > > > > > > >> [email protected] <mailto:[email protected]> <mailto: > > [email protected] > > > > > > > <mailto: > > > > > > > > [email protected]>> http://db.apache.org/jdo < > > > > http://db.apache.org/jdo> > > > > > < > > > > > > > > >> http://db.apache.org/jdo <http://db.apache.org/jdo>> > > > > > > > > > > > > > > > > Craig L Russell > > > > > > > > Secretary, Apache Software Foundation > > > > > > > > [email protected] <mailto:[email protected]> > > http://db.apache.org/jdo > > > < > > > > > > > > http://db.apache.org/jdo> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Sönke Liebau > > > Partner > > > Tel. +49 179 7940878 > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany > > > > > > > > -- > Sönke Liebau > Partner > Tel. +49 179 7940878 > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany >
