ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikolay Izhikov <nizhi...@apache.org>
Subject Re: Peer review: Victory over Patch Available debt
Date Mon, 18 Mar 2019 13:47:29 GMT
Dmitriy.

Actually, I doesn't understand your last mail.
Do we have some issue to solve?
If yes, can you write it down?

What fixes need to be reviewed?


пн, 18 мар. 2019 г. в 16:31, Dmitriy Pavlov <dpavlov@apache.org>:

> I'm not second-guessing someone's motivation. And without a particular
> case, it is not reasonable to discuss reasons why a patch is not merged.
>
> Silence=agreement in case there was clearly stated about it. Lazy consensus
> is simply an announcement of 'silence gives assent.' When someone wants to
> determine the sense of the community this way, it might do so with a mail
> message such as:
> "The patch below fixes bug #8271847; if no-one objects within three days,
> I'll assume lazy consensus and commit it."
>
> We used this approach from time to time, and we do have a situation when
> nobody wants to review even a small fix.
>
> It is as simple as this:
> - Any contributor (even non-committer) may come and say I will review.
> - Only committers can merge. So it is not reasonable to call to Lazy
> consensus if you are not a committer.
> - PMCs may revert by veto.
>
> We can remove any words from HTC because of reasonable concerns of
> misunderstanding, but policy still can be used.
>
>
> пн, 18 мар. 2019 г. в 13:52, Anton Vinogradov <av@apache.org>:
>
> > In case nobody cares, most likely we have a problem with a contribution
> or
> > motivation, not with lazy committers :)
> > Please remove the "lazy" phrase, since it can be interpreted as "silence
> as
> > an agreement" which is always not true.
> >
> > On Mon, Mar 18, 2019 at 1:13 PM Dmitriy Pavlov <dpavlov@apache.org>
> wrote:
> >
> > > Hi, thank you all for your replies, I'm happy we discussing it, so we
> > could
> > > clearly understand this policy and how to apply it.
> > >
> > > a committer will always merge the change, was it approved by another
> > > contributor/committer/lazy consensus/vote - does not matter. And a
> > > committer will be responsible to take a final decision.
> > >
> > > There will no any kind of automatic merge.
> > >
> > > If a maintainer is on vacation, some other contributor may come to the
> > > thread and say: Hi, please wait for a review from xxx. Any kind of
> > > discussion != silence. And lazy consensus is a way to apply change when
> > > absolutely nobody (except the author) cares about it.
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > пн, 18 мар. 2019 г. в 12:37, Nikolay Izhikov <nizhikov@apache.org>:
> > >
> > > > Hello, Vladimir.
> > > >
> > > > Thanks for the detailed answer.
> > > > I think your statement doesn't differs with Dmitry statement much.
> > > > Do we have committer who merge without confidence in patch content?
> > > > If yes, they should stop to do it.
> > > >
> > > >
> > > > пн, 18 мар. 2019 г. в 12:00, Anton Vinogradov <av@apache.org>:
> > > >
> > > > > Huge +1 to "We should stress out that a patch should be committed
> if
> > > and
> > > > > only if committer is confident with the changes."
> > > > >
> > > > > On Mon, Mar 18, 2019 at 11:54 AM Vladimir Ozerov <
> > vozerov@gridgain.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This is tough question, and first of all I'd like to ask
> > participants
> > > > to
> > > > > > keep cold head. This is a public question and can be discussed
on
> > the
> > > > dev
> > > > > > list safely.
> > > > > >
> > > > > > On the one hand, it is true that a number of patches are not
> > reviewed
> > > > > for a
> > > > > > long time, what negatively affects community development. On
the
> > > other
> > > > > > hand, we definitely do not want to sacrifice product quality
only
> > > > because
> > > > > > e.g. responsible component owner was on a sick leave or vacation
> > and
> > > > was
> > > > > > not able to review the patch in a timely manner. Some compromise
> is
> > > > > needed.
> > > > > >
> > > > > > IMO additional comments in HTC may solve the issue. We should
> > stress
> > > > out
> > > > > > that a patch should be committed if and only if committer is
> > > confident
> > > > > with
> > > > > > the changes. Confidence comes from either experience (you worked
> > with
> > > > > > component a lot and know what you are doing), or from review
by
> > > > > component's
> > > > > > expert. But if there is an outdated patch and you are not
> confident
> > > > > enough,
> > > > > > just don't merge. Let is stay in Patch Available as long as
> needed.
> > > > > >
> > > > > > In case of lazy consensus we may ask committers to add comments
> to
> > > the
> > > > > > ticket explaining why they decided to merge a ticket without
> > expert's
> > > > > > review. This should help us avoid bad commits.
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > > > On Mon, Mar 18, 2019 at 11:33 AM Anton Vinogradov <av@apache.org
> >
> > > > wrote:
> > > > > >
> > > > > > > Dmitry,
> > > > > > >
> > > > > > > Phrase "Code modifications can be approved by silence:
by lazy
> > > > > consensus
> > > > > > > (72h) after Dev.List announcement." looks unacceptable
to me.
> > > > > > >
> > > > > > > Please roll back the changes and start the discussion at
the
> > > private
> > > > > list
> > > > > > > and never do such updates in the future without the discussion.
> > > > > > >
> > > > > > > On Fri, Mar 15, 2019 at 8:29 PM Dmitriy Pavlov <
> > dpavlov@apache.org
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Igniters,
> > > > > > > >
> > > > > > > > sorry for the late reply. Because this process time
to time
> > > causes
> > > > > > > > questions, I decided to add a couple of words to our
wiki.
> > > > > > > >
> > > > > > > > I've added topics about peer review to HTC
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-PeerReviewandLGTM
> > > > > > > >
> > > > > > > > Actually, it is (more or less) rules of Apache Beam
project,
> as
> > > > well
> > > > > as
> > > > > > > > Apache Training(incubating), as well as our current
process +
> > > > Apache
> > > > > > > > policies.
> > > > > > > >
> > > > > > > > Sincerely,
> > > > > > > > Dmitriy Pavlov
> > > > > > > >
> > > > > > > >
> > > > > > > > чт, 16 авг. 2018 г. в 17:46, Yakov Zhdanov
<
> > yzhdanov@apache.org
> > > >:
> > > > > > > >
> > > > > > > > > Dmitry,
> > > > > > > > >
> > > > > > > > > I like your suggestion very much! And I want
everyone to
> > > follow.
> > > > > > Let's
> > > > > > > > see
> > > > > > > > > if it helps.
> > > > > > > > >
> > > > > > > > > Can I ask everyone who has submitted tickets
for review to
> > add
> > > a
> > > > > > > comment
> > > > > > > > > described by Dmitry to each ticket submitted
and see if any
> > > > > > additional
> > > > > > > > > check is still required and fix remaining issues?
I believe
> > > this
> > > > > > should
> > > > > > > > > speed up review process very much.
> > > > > > > > >
> > > > > > > > > --Yakov
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message