Stephen, On Thu, Apr 16, 2015 at 4:21 PM, Ma, Stephen B. <stephen...@hp.com> wrote: > As it stands currently, neutron on the stable/juno branch behaves as if > https://review.openstack.org/#/c/164329/ was never merged. The merge of > patch https://review.openstack.org/#/c/153181 puts some LOG.debug > statements in the for-loop of the same function. The net result is the > unintentional revert of patch https://review.openstack.org/#/c/164329/.
I wouldn't call this a revert. Instead, it brought back some context that the original patch dealt with but that the cherry pick did not. This is a regression but not a revert of the patch. > According to review.openstack.org, the patch > https://review.openstack.org/#/c/153181 merged on April 1st at 1:56 pm. > Patch https://review.openstack.org/#/c/164329/ merged on the same day at > 2:21 pm. After 153181 merged, the review of 164329 should have triggered a > merge conflict error which would have compelled the owner to do a rebase. > The merge conflict wasn’t reported and the patches merged anyway. Does this > point to a problem with the Jenkins CI system? This was not a problem with the CI system. The infrastructure behaved exactly as it should. The problem happened when [4] was proposed with a lesser scope than [2] and then [3] merged making that extra scope relevant again. I understand why the scope was reduced. It is often necessary to react to changes in context when cherry picking. That is the nature of cherry picking, bringing a patch in to a new context and reacting. It was unfortunate that [3] brought back the relevant context which made the broader scope of the change in [2] necessary and then they merged together. After having given it some thought, I have come to the conclusion that the only way to catch this would have been to include a proper test with [2] and [4] that log debug statements are not executed in the loop. Such a test would have kicked out whichever of [3] and [4] happened to merge last from the gate. It would've been really annoying to fail in the gate but it would have flagged the problem then and there. Really, the failure was in not requiring the tests necessary to prevent regression. Until we add these tests, we will be vulnerable to this regression again. For example, I could merge a new patch to add more logging to this loop today and we'd be back to square one. Carl [1] https://review.openstack.org/#/c/147455 [2] https://review.openstack.org/#/c/149784 [3] https://review.openstack.org/#/c/153181 [4] https://review.openstack.org/#/c/164329 __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
