openstack team mailing list archive
-
openstack team
-
Mailing list archive
-
Message #14416
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