← Back to team overview

openstack team mailing list archive

Re: Don't bypass code-review.

 

On Tue, 2012-07-10 at 15:32 -0400, Eric Windisch wrote:
> > 
> > Jenkins was failing to merge *ANY* code reviews for openstack-common.
> > The root of the dependency tree of your patch sets is insufficient to
> > make the test suite pass. The set of patches needed to go in in one
> > commit. I was ping'd on IRC to check into the tests that were failing
> > when the build hosts got pyzmq installed on them by another patch set. 
> > 
> > I'm sorry that I bypassed the code review, but in light of nothing
> > getting through due to the ZeroMQ tests being broken to begin with,
> > there was not much I could do. The RPC bits got added in prior to test
> > gating, and further these tests only trigger when pyzmq is installed.
> > 
> I understand the dilemma, but it wasn't like I didn't have reviews in
> progress to address this.  If the patches weren't sufficient,
> contacting me or giving me a -1 would have been a better option.
> Alternatively, you could have investigated why pyzmq might have gotten
> installed, and seen if it could have been disabled.  Instead, you
> merged your change 30 minutes before contacting the mailing list or
> otherwise attempting communication.  It took, what, 10 minutes for me
> to receive, read, and reply to your post?

I asked on irc if anyone was interested in the ZeroMQ code prior to
"fixing" it. But I realize I am at fault on this one.

> I've already pushed an updated change that collapses the changes into
> a larger patch that should get Jenkins going again (tests pass
> locally, we'll see how Jenkins feels about it)
> 
> Right now, I'd just like to get past this. I'm sorry if I've been at
> all too rough on you.
> 
> I'd just appreciate that in the future, even if the build is broken,
> that code review is not bypassed.  Additionally, if there is a
> reasonable way to approach the author of code, especially if there is
> already a patch in review, that opposing patches aren't shoe-horned in
> without review or oversight.

Agreed. Perhaps we should update each file with contact information for
the responsible parties for each module openstack-common wide since many
times the author of the code is not that who shows up in git blame.

Happy Hacking!

7-11



References