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

Re: PR Reviews


Thanks James for driving this topic, summarizing, and documenting.

Ed

On Tue, Sep 11, 2018 at 8:42 AM Sendoro Juma <sendoro@singo.africa> wrote:

> +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
> > > > > > >
> > > > > > >
> > > > > > >     >
> > > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> >
>
>
>
>
>
>

-- 
*Ed Cable*
President/CEO, Mifos Initiative
edcable@xxxxxxxxx | Skype: edcable | Mobile: +1.484.477.8649

*Collectively Creating a World of 3 Billion Maries | *http://mifos.org
<http://facebook.com/mifos>  <http://www.twitter.com/mifos>