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 <k...@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
