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 >
