OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PR Reviews


+1




On Tue, Sep 11, 2018 at 5:12 PM +0200, "James Dailey" <jamespdailey@xxxxxxxxx> wrote:










I put the content here:
https://cwiki.apache.org/confluence/display/FINERACT/Committer%27s+Zone



On Tue, Sep 11, 2018 at 1:43 AM Myrle Krantz  wrote:

> Hey all,
>
> I believe that James has correctly captured the consensus and would
> encourage him to document that.
>
> James if you need any help finding a good spot in the wiki for that, just
> shout.
>
> Best Regards,
> Myrle
> On Tue, Sep 11, 2018 at 8:47 AM Sendoro Juma  wrote:
> >
> > +1
> >
> >
> >
> >
> > For document... Any change improvement will be done in the document
> page...
> >
> >
> >
> >
> > I would also like the page to indicate...
> >
> >
> >
> >
> > On Tue, Sep 11, 2018 at 4:40 AM +0200, "James Dailey" <
> jamespdailey@xxxxxxxxx> wrote:
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > Hi all - I will comment that I think this is directionally great.
> >
> > As Sendoro suggested:
> > 1) The dev A should try to ask a specific person(s) to review if they
> can,
> > as in dev B, and to anyway give the list some time to review the PR.
> Then
> > 72 hrs later or so, they're able to commit their own PR under a principle
> > of lazy consensus.  This also assumes that people are fully discussing
> > their ideas for a PR in advance and there is a jira issue where the
> > requirements are documented.
> >
> > 2) The Committer may decide that the proposed PR is significant enough
> that
> > they really want someone to review and they can flag that too.
> >
> > Also, the other part of my suggestion (and the really cool insight at
> > Apache) was that:
> > 3) Members of the community who are NOT yet committers would be welcome
> to
> > pitch in on the review, thus earning karma. This creates a dynamic where
> > devs can give feedback and quality review without taking on a task of dev
> > themselves. +1 for the reviewer.
> >
> >
> > I also think Myrle is right in that we don't need a vote, but hope that
> > others will weigh in and consider so we can have a sense of consensus.
> If
> > not, our lazy consensus ideal is that we're in agreement...but first
> let's
> > keep discussion going.
> >
> > I'm happy to document the above on wiki if that's useful.  Not sure what
> > else is needed aside from the 1, 2, 3.  Thoughts?
> >
> > James
> >
> > On Wed, Aug 29, 2018 at 11:05 AM Myrle Krantz  wrote:
> >
> > > Hi all,
> > >
> > > I suggest we give people a chance to give input before we complete the
> > > decision making process.  If we converge on common ground we may not
> need
> > > to vote at all.  We may just need someone to summarize the consensus
> and
> > > document it in wiki.
> > >
> > > I like Sendoro's suggestion.  I don't see a need to hurry PR's merges
> to
> > > faster than 72 hours, and I'd like people to look at each other's code
> to
> > > increase quality, but if we can't get people to do that, it shouldn't
> block
> > > progress.
> > >
> > > Best Regards,
> > > Myrle
> > >
> > >
> > > On Wed 29. Aug 2018 at 08:03 Mexina Daniel  wrote:
> > >
> > > > Hello Ed
> > > >
> > > > Thank you for the prompt reply,
> > > >
> > > > So do we have to vote for James's proposal or we can implement it
> from
> > > now
> > > > on?
> > > >
> > > > Hello Sendoro
> > > >
> > > > Thanks for more inputs,
> > > >
> > > > Adding to that, i think even non-committers should be involved in
> > > > reviewing i.e non-committer A can review to any submitted PR that
> hasn't
> > > > started to be reviewed.
> > > >
> > > > Best Regards
> > > >
> > > > > On August 28, 2018 at 11:06 PM Sendoro Juma
> > > > wrote:
> > > > >
> > > > >
> > > > >     Dear Ed, Mexina,
> > > > >
> > > > >
> > > > >     I would wish we meet at the middle...  '"Peer Review",  i.e.
> > > > committer A, ask committer B reviewing his/her PR. and vice versa...
> > > > >
> > > > >
> > > > >     However, as rule like you said if not happen in 72 hours.. then
> > > > Committer A can proceed especially when s/he is sure Quality  of
> his/her
> > > PR.
> > > > >
> > > > >
> > > > >     How about that?
> > > > >
> > > > >         > >
> > > > > >         On August 28, 2018 at 7:16 PM Ed Cable
> > > > wrote:
> > > > > >
> > > > > >         Mexina,
> > > > > >
> > > > > >         Thank you for picking up on James' thread and putting it
> into
> > > > action with
> > > > > >         some spot-on questions.
> > > > > >
> > > > > >         Please see my replies inline:
> > > > > >
> > > > > >         On Tue, Aug 28, 2018 at 6:46 AM Mexina Daniel
> > > >  wrote:
> > > > > >
> > > > > >             > > >
> > > > > > >             Hello fineract'ers
> > > > > > >
> > > > > > >             Can someone help me to understand few things:
> > > > > > >
> > > > > > >                1. Should the PR be reviewed by someone (who
> can be
> > > a
> > > > committer or not)
> > > > > > >                   before being merged or a committer can merge
> > > > his/her PR even if it hasn't
> > > > > > >                   being reviewed by anyone?
> > > > > > >
> > > > > > >         > >
> > > > > >         In the spirit of James' email requesting a cultural
> change
> > > > around PRs and
> > > > > >         the feedback from Ross on minimizing any barriers to
> > > > contribution, James
> > > > > >         had proposed and I vouch we adopt a lazy consensus policy
> > > > around committers
> > > > > >         being able to merge their own PRs - if nobody objects
> within
> > > > 72 hours, the
> > > > > >         committer can merge their own PR. As James puts, if it
> > > breaks,
> > > > it can be
> > > > > >         unmerged. This would be a shift from our currently
> policy in
> > > > which all PRs
> > > > > >         (from everyone including committers) require a review
> before
> > > > being merged.
> > > > > >
> > > > > >         >
> > > > > >
> > > > > >             > > >                1. Do we have reviewers of PRs
> (If
> > > > yes, i would like to know even few of
> > > > > > >                   them) or anyone can review?
> > > > > > >
> > > > > > >         > >
> > > > > >         Historically we only allowed committers to review PRs but
> > > > based on Ross'
> > > > > >         insight into some of the breakthrough changes other
> projects
> > > > recently made,
> > > > > >         anybody can now review a PR (committer or not). It would
> be
> > > > great to have a
> > > > > >         dedicated team of reviewers but there is no barrier to
> > > > reviewing PRs so we
> > > > > >         encourage anybody to review and comment on PRs - it's a
> great
> > > > way to build
> > > > > >         merit and pave path towards becoming a committer.
> > > > > >
> > > > > >         Ed
> > > > > >
> > > > > >         >
> > > > > >
> > > > > >             > > >
> > > > > > >             Best Regards
> > > > > > >
> > > > > > >             Mexina
> > > > > > >
> > > > > > >         > >
> > > > > >         --
> > > > > >         *Ed Cable*
> > > > > >         President/CEO, Mifos Initiative
> > > > > >         edcable@xxxxxxxxx | Skype: edcable | Mobile:
> +1.484.477.8649 <(484)%20477-8649>
> > > <(484)%20477-8649>
> > > > > >
> > > > > >         *Collectively Creating a World of 3 Billion Maries | *
> > > > http://mifos.org
> > > > > >
> > > > > >
> > > > > >     >
> > > >
> > >
> >
> >
> >
> >
> >
>