Re: Peer review: Victory over Patch Available debt
I've removed debated line from the document. As we don't have consensus on lazy consensus it is better to start a separate discussion for this topic. I prefer to build consensus here while procedural changes do not require it and Majority Approval is enough. We time to time use lazy consensus, so it is not true that a change can't be approved by silence. This policy can be perfectly combined with RTC (proved by Beam and Training(incubating) ). Our preferences for CTR/RTC usage for different modules quite a huge discussion, which may end up/require our mainteners list review. If some one would like to do it, please go ahead. But it is out of the scope of initial proposal of peer review. пн, 18 мар. 2019 г. в 17:21, Dmitriy Pavlov : > Nikolay, sorry for going off-topic of the current thread. Just a few > examples: > https://issues.apache.org/jira/browse/IGNITE-8376 > https://issues.apache.org/jira/browse/IGNITE-11521 > https://issues.apache.org/jira/browse/IGNITE-11397 > https://issues.apache.org/jira/browse/IGNITE-11346 > https://issues.apache.org/jira/browse/IGNITE-11337 > > Maybe some contributions there have some issues, but in this case, we can > be confident to press 'Cancel Patch' > > Sincerely, > > > > пн, 18 мар. 2019 г. в 16:47, Nikolay Izhikov : > >> 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 : >> >> > 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 : >> > >> > > 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 >> > 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 : >> > > > >> > > > > 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 : >> > > > > >> > > > > > 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 < >> > > voze...@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
Re: Peer review: Victory over Patch Available debt
Nikolay, sorry for going off-topic of the current thread. Just a few examples: https://issues.apache.org/jira/browse/IGNITE-8376 https://issues.apache.org/jira/browse/IGNITE-11521 https://issues.apache.org/jira/browse/IGNITE-11397 https://issues.apache.org/jira/browse/IGNITE-11346 https://issues.apache.org/jira/browse/IGNITE-11337 Maybe some contributions there have some issues, but in this case, we can be confident to press 'Cancel Patch' Sincerely, пн, 18 мар. 2019 г. в 16:47, Nikolay Izhikov : > 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 : > > > 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 : > > > > > 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 > > 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 : > > > > > > > > > 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 : > > > > > > > > > > > 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 < > > > voze...@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
Re: Peer review: Victory over Patch Available debt
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 : > 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 : > > > 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 > 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 : > > > > > > > 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 : > > > > > > > > > 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 < > > voze...@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 > > > > > 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
Re: Peer review: Victory over Patch Available debt
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 : > 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 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 : > > > > > 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 : > > > > > > > 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 < > voze...@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 > > > 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 < > dpav...@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.
Re: Peer review: Victory over Patch Available debt
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 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 : > > > 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 : > > > > > 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 > > > > 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 > > 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 > > > > > 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 >: > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: Peer review: Victory over Patch Available debt
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 : > 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 : > > > 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 > > 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 > 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 > > > 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 : > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > >
Re: Peer review: Victory over Patch Available debt
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 : > 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 > 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 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 > > 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 : > > > > > > > > > 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 > > > > > > > > > > > > > > >
Re: Peer review: Victory over Patch Available debt
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 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 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 > 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 : > > > > > > > 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 > > > > > > > > > >
Re: Peer review: Victory over Patch Available debt
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 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 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 : > > > > > 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 > > > > > >
Re: Peer review: Victory over Patch Available debt
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 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 : > > > 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 > > >
Re: Peer review: Victory over Patch Available debt
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 : > 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 >
Re: Peer review: Victory over Patch Available debt
Great idea! Review is not only meant for the committers, but for everyone in the community. Committers are only responsible for final reviews and merges. D. On Thu, Aug 16, 2018 at 7:29 AM, Dmitriy Pavlov wrote: > Hi Igniters, > > For a long time review queue in the community is quite long. Ticket count > is around 80-100 for last year. I would like to make the proposal, which > should help the community with this aspect. Being reviewer for some tickets > I found a number of tickets from new community members with not checked > tests, not linked dev. list discussion, empty description, code style > violations etc. It is not easy to finalize the review in such case. > > Every Igniter could make way too needed contribution to Apache Ignite by > educating newcomers how to make ticket, patch, and discussion as perfect as > it is humanly possible. > > I propose any Igniter (regardless of his role and experience: PMC, > committer or contributor) will comment patches of other members in JIRA to > help newcomers make contribution compliant with > https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute I > know a number of community members already do so. > > Every contributor could find any ticket he or she likes in > https://cwiki.apache.org/confluence/display/IGNITE/ > Issues+waiting+for+review > > > The contributor can check compliance using > https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist > and copy-paste review checklist items to the ticket with his or her visa > about each item. Each contribution can have an infinite number of visas. > > Looks good visa are also helpful, but I would like to go forward and > formalize this process. Please share your vision. I would be happy to run a > short webinar in case you have questions. I will update how to contribute > shortly if we agree. > > Sincerely, > Dmitriy Pavlov >
Peer review: Victory over Patch Available debt
Hi Igniters, For a long time review queue in the community is quite long. Ticket count is around 80-100 for last year. I would like to make the proposal, which should help the community with this aspect. Being reviewer for some tickets I found a number of tickets from new community members with not checked tests, not linked dev. list discussion, empty description, code style violations etc. It is not easy to finalize the review in such case. Every Igniter could make way too needed contribution to Apache Ignite by educating newcomers how to make ticket, patch, and discussion as perfect as it is humanly possible. I propose any Igniter (regardless of his role and experience: PMC, committer or contributor) will comment patches of other members in JIRA to help newcomers make contribution compliant with https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute I know a number of community members already do so. Every contributor could find any ticket he or she likes in https://cwiki.apache.org/confluence/display/IGNITE/Issues+waiting+for+review The contributor can check compliance using https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist and copy-paste review checklist items to the ticket with his or her visa about each item. Each contribution can have an infinite number of visas. Looks good visa are also helpful, but I would like to go forward and formalize this process. Please share your vision. I would be happy to run a short webinar in case you have questions. I will update how to contribute shortly if we agree. Sincerely, Dmitriy Pavlov
