<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Dec 29, 2013 at 5:04 AM, Sean Dague <span dir="ltr"><<a href="mailto:sean@dague.net" target="_blank">sean@dague.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 12/29/2013 03:06 AM, Day, Phil wrote:<br>
<snip><div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Basically, I'm not sure what problem you're trying to solve - lets tease that<br>
out, and then talk about how to solve it. "Backwards incompatible change<br>
landed" might be the problem - but since every reviewer knew it, having a<br>
longer review period is clearly not connected to solving the problem :).<br>
<br>
</blockquote>
<br>
That assumes that a longer review period wouldn't of allowed more reviewers to provide input - and I'm arguing the opposite. I also think that some clear guidelines might have led to the core reviewers holding this up for longer. As I said in my original post, the intent to get more input was clear in the reviews, but the period wasn't IMO long enough to make sure all the folks who may have something to contribute could. I'd rather see some established guidelines than have to be constantly on the lookout for changes every day or so and hoping to catch them in time.<br>
</blockquote>
<br></div>
Honestly, there are currently 397 open reviews in Nova. I am not convinced that waiting on this one would have come up with a better decision. I'll given an alternative point of view of the graceful shutdown patch, where we sat on that for months, had many iterations, landed it, it added 25 minutes to all the test runs (which had been hinted at sometime in month 2 of the review, but got lots in the mists of time), and we had to revert it.<br>
<br>
I'm not convinced more time brings more wisdom. We did take it to the list, and there were no objections. I did tell Robert to wait because I wanted to get those points of view. But they didn't show up. Because it was holidays could we have waited longer? Sure. I'll take a bad on that in feeling that Dec 19th wasn't really holidays yet because I was still working. :) But, honestly, given no negative feedback on the thread in question and no -1 on the review, the fact that folks like google skipped ext3 entirely, means this review was probably landing regardless.<br>
<br>
Every time we need to do a revert, we need to figure out how to catch it the next time. "Humans be better" is really not a solution. So this sounds like we need a guest compatibility test where we boot a ton of different guests on each commit and make sure they all work. I'd whole heartily support getting that in if someone wants to champion that. That's really going to be the only way we have a systematic way of knowing that we break SLES in the future.<br>
<br>
So the net, we're all human, and sometimes make mistakes. I don't think we're going to fix this with review policy changes, but we could with actual CI enhancements.</blockquote><div><br></div><div>++, I think this sums up things very well. Leaving a review open for longer makes the somewhat incorrect assumption that eventually the right person would review this. Perhaps that assumption is true for some people but I doubt its true for everyone (it's definitely not for me). Every time an issue like this gets through we should be asking ourselves how can we make the gate detect this next time? (That question is why I wrote the large-ops test.) It sounds like we have two leads on how to prevent this in the future:</div>
<div><br></div><div>* Guest compatibility testing.</div><div>* Better backwards compatibility/upgrade testing (As a later email mentions).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
<br>
-Sean<br>
<br>
-- <br>
Sean Dague<br>
Samsung Research America<br>
<a href="mailto:sean@dague.net" target="_blank">sean@dague.net</a> / <a href="mailto:sean.dague@samsung.com" target="_blank">sean.dague@samsung.com</a><br>
<a href="http://dague.net" target="_blank">http://dague.net</a></font></span><div class="HOEnZb"><div class="h5"><br>
<br>
______________________________<u></u>_________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
</div></div></blockquote></div><br></div></div>