We are speaking in this thread about two reviewers on big PR, but there are PRs without any reviewers =)
What contributor can do if his PR have not been reviewed for 4 week? For example https://github.com/apache/zeppelin/pull/2684 Thanks, Maksim Belousov -----Original Message----- From: Jongyoul Lee [mailto:jongy...@gmail.com] Sent: Tuesday, December 19, 2017 8:34 PM To: dev <dev@zeppelin.apache.org> Subject: Re: [DISCUSS] Review process I agree with some large PR should be delayed a bit longer. What I meant is we don't have to wait for all kind of PRs. On Wed, Dec 20, 2017 at 2:11 AM, Felix Cheung <felixcheun...@hotmail.com> wrote: > +1 > What would be the rough heuristic people will be comfortable with- > what is small and what is big? > > _____________________________ > From: Anthony Corbacho <anthonycorba...@apache.org> > Sent: Monday, December 18, 2017 3:02 PM > Subject: Re: [DISCUSS] Review process > To: <dev@zeppelin.apache.org> > > > I think for large PR (new feature or big change) we should still keep > more than one approval before merging it since this will require more > attension. > > But for bug fix i think one approval should be enough. > > On Tue, 19 Dec 2017 at 7:49 AM Jeff Zhang <zjf...@gmail.com> wrote: > > > Agree with @Felix, especially for the large PR and PR of new > > features it is still necessary to have more than +1. > > > > I think committer have the ability to identity whether this PR is > > complicated enough that needs another committer's review. As long we > > as have consensus, we could commit some PR without delay and some PR > > for > more > > reviews. So that we can balance the development speed and code quality. > > > > > > > > Miquel Angel Andreu Febrer > > <miquelangeland...@gmail.com>于2017年12月19日周二 > > 上午2:07写道： > > > > > You can automate that process in jenkins and manage the delay time > > > of merging a pull request > > > > > > El 18 dic. 2017 18:03, "Felix Cheung" <felixcheun...@hotmail.com> > > > escribió: > > > > > > > I think it is still useful to have a time delay after one > > > > approve > since > > > > often time there are very feedback and updates after one > > > > committer > > > approval. > > > > > > > > Also github has a tab for all PRs you are subscribed to, it > > > > shouldn’t > > be > > > > very hard to review all the approved ones again. > > > > > > > > ________________________________ > > > > From: Jongyoul Lee <jongy...@gmail.com> > > > > Sent: Monday, December 18, 2017 8:04:51 AM > > > > To: dev@zeppelin.apache.org > > > > Subject: Re: [DISCUSS] Review process > > > > > > > > Good for summary. But actually, no committer merges without > > > > delay > after > > > > reviewing it. So I thought we should clarify it officially. > > > > > > > > Now, some committers, including me, will be able to merge some > > > > PRs > > > without > > > > delay and burden. > > > > > > > > On Mon, 18 Dec 2017 at 11:27 PM moon soo Lee <m...@apache.org> > wrote: > > > > > > > > > Hi, > > > > > > > > > > Current review process[1] does require either at least a +1 > > > > > from > > > > committer > > > > > or 24 hours for lazy consensus. > > > > > > > > > > Pullrequest can be open for 1 or 2 days for additional review, > > > > > but > i > > > > think > > > > > they're not hard requirements. (e.g. Hotfixes are already > > > > > being > > merged > > > > > without waiting additional review) > > > > > > > > > > So, technically, current policy allows any committer can start > > review, > > > > mark > > > > > +1 and merge immediately without any delay if necessary. > > > > > > > > > > Thanks, > > > > > moon > > > > > > > > > > [1] > > > > > > > > > > http://zeppelin.apache.org/contribution/contributions. > > > > html#the-review-process > > > > > > > > > > > > > > > On Mon, Dec 18, 2017 at 2:13 AM Belousov Maksim Eduardovich < > > > > > m.belou...@tinkoff.ru> wrote: > > > > > > > > > > > +1 for non-delay merging. > > > > > > Our team have opened approved PR [1] for 5 days. > > > > > > > > > > > > I didn't find any pages with `consensus how to review and > > > > > > merge contributions`. > > > > > > It would be nice to write a check list for reviewer. > > > > > > > > > > > > The development of Zeppelin is very important for us and we > > > > > > want > to > > > > > review > > > > > > new commits. > > > > > > > > > > > > > > > > > > [1] https://github.com/apache/zeppelin/pull/2697 > > > > > > > > > > > > > > > > > > Thanks, > > > > > > Maksim Belousov > > > > > > > > > > > > -----Original Message----- > > > > > > From: Jongyoul Lee [mailto:jongy...@gmail.com] > > > > > > Sent: Monday, December 18, 2017 12:12 PM > > > > > > To: dev <dev@zeppelin.apache.org> > > > > > > Subject: Re: [DISCUSS] Review process > > > > > > > > > > > > Thank you for the replying it. I think so > > > > > > > > > > > > On Mon, Dec 18, 2017 at 3:15 PM, Miquel Angel Andreu Febrer > > > > > > < miquelangeland...@gmail.com> wrote: > > > > > > > > > > > > > I agree, ig is necessary to have no delay afternoon > > > > > > > merging. I > > > think > > > > > > > it will help speed up processes and help contributors > > > > > > > > > > > > > > El 18 dic. 2017 4:33, "Jongyoul Lee" <jongy...@gmail.com> > > > escribió: > > > > > > > > > > > > > > Hi committers, > > > > > > > > > > > > > > I want to suggest one thing about our reviewing process. > > > > > > > We > have > > > the > > > > > > > policy to wait for one-day before merging some PRs. AFAIK, > > > > > > > It's because we reduce mistakes and prevent abuses from > > > > > > > committing > by > > > > owner > > > > > > > without reviewing it concretely. I would like to change > > > > > > > this > > policy > > > > to > > > > > > > remove delay after merging it. We, recently, don't have > > > > > > > much > > > > reviewers > > > > > > > and committers who can merge continuously, and in my case, > > > > > > > I, sometimes, forget some PRs that I have to merge. And I > > > > > > > also > > believe > > > > > > > all committers have consensus how to review and merge > > > contributions. > > > > > > > > > > > > > > How do you think of it? > > > > > > > > > > > > > > JL > > > > > > > > > > > > > > -- > > > > > > > 이종열, Jongyoul Lee, 李宗烈 > > > > > > > http://madeng.net > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > 이종열, Jongyoul Lee, 李宗烈 > > > > > > http://madeng.net > > > > > > > > > > > > > > > -- > > > > 이종열, Jongyoul Lee, 李宗烈 > > > > http://madeng.net > > > > > > > > > > > > -- 이종열, Jongyoul Lee, 李宗烈 http://madeng.net
