spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Henry Saputra <henry.sapu...@gmail.com>
Subject Re: -1s on pull requests?
Date Mon, 21 Jul 2014 21:19:37 GMT
There is ASF guidelines about Voting, including code review for
patches: http://www.apache.org/foundation/voting.html

Some ASF project do three +1 votes are required (to the issues like
JIRA or Github PR in this case) for a patch unless it is tagged with
lazy consensus [1] of like 48 hours.
For patches that are not critical, waiting for a while to let some
time for additional committers to review would be the best way to go.

Another thing is that all contributors need to be patience once their
patches have been submitted and pending reviewed. This is part of
being in open community.

Hope this helps.


- Henry

[1] http://www.apache.org/foundation/glossary.html#LazyConsensus

On Mon, Jul 21, 2014 at 1:59 PM, Patrick Wendell <pwendell@gmail.com> wrote:
> I've always operated under the assumption that if a commiter makes a
> comment on a PR, and that's not addressed, that should block the PR
> from being merged (even without a specific "-1"). I don't know of any
> cases where this has intentionally been violated, but I do think this
> happens accidentally some times.
>
> Unfortunately, we are not allowed to use those github hooks because of
> the way the ASF github integration works.
>
> I've lately been using a custom-made tool to help review pull
> requests. One thing I could do is add a feature here saying which
> committers have said LGTM on a PR (vs the ones that have commented).
> We could also indicate the latest test status as Green/Yellow/Red
> based on the Jenkins comments:
>
> http://pwendell.github.io/spark-github-shim/
>
> As a warning to potential users, my tool might crash your browser.
>
> - Patrick
>
> On Mon, Jul 21, 2014 at 1:44 PM, Kay Ousterhout <keo@eecs.berkeley.edu> wrote:
>> Hi all,
>>
>> As the number of committers / contributors on Spark has increased, there
>> are cases where pull requests get merged before all the review comments
>> have been addressed. This happens say when one committer points out a
>> problem with the pull request, and another committer doesn't see the
>> earlier comment and merges the PR before the comment has been addressed.
>>  This is especially tricky for pull requests with a large number of
>> comments, because it can be difficult to notice early comments describing
>> blocking issues.
>>
>> This also happens when something accidentally gets merged after the tests
>> have started but before tests have passed.
>>
>> Do folks have ideas on how we can handle this issue? Are there other
>> projects that have good ways of handling this? It looks like for Hadoop,
>> people can -1 / +1 on the JIRA.
>>
>> -Kay

Mime
View raw message