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

Re: PR Reviews


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 <myrle@xxxxxxxxxx> 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 <mexina@singo.africa> 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 <sendoro@singo.africa>
> > 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 <edcable@xxxxxxxxx>
> > 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
> > <mexina@singo.africa> 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>
> > > >
> > > >         *Collectively Creating a World of 3 Billion Maries | *
> > http://mifos.org
> > > >         <http://facebook.com/mifos> <http://www.twitter.com/mifos>
> > > >
> > > >     >
> >
>