[openstack-dev] When to revert a patch?

Flavio Percoco flavio at redhat.com
Fri Mar 4 17:44:54 UTC 2016


On 04/03/16 10:24 -0500, Morgan Fainberg wrote:
>
>On Mar 4, 2016 10:16, "Monty Taylor" <mordred at inaugust.com> wrote:
>>
>> On 03/04/2016 08:37 AM, Ruby Loo wrote:
>>>
>>> Hijacked from ' [openstack-dev] [ironic] Remember to follow RFE process'
>>> thread:
>>>
>>>         > Should we revert the patch [1] for now? (Disclaimer. I haven't
>looked at the
>>>         > patch itself. But I don't think I should have to, to know what the
>API
>>>         > change is.)
>>>         >
>>>
>>>         Thanks for calling it out Ruby, that's unfortunate that the
>>>         patch was
>>>         merged without the RFE being approved. About reverting the patch I
>>>         think we shouldn't do that now because the patch is touching the API
>>>         and introducing a new microversion to it.
>>>
>>>
>>>     Exactly. I've -2'ed the revert, as removing API version is even
>>>     worse than landing a change without an RFE approved. Let us make
>>>     sure to approve RFE asap, and then adjust the code according to it.
>>>
>>>
>>> This brings up another issue, which I recall discussing before. Did we
>>> decide that we'd never revert something that touches the
>>> API/microversion? It might be good to have guidelines on this if we
>>> don't already. IF the API is incorrect? If the API could be improved? If
>>> the API was only in master for eg 48 hours?
>>
>>
>> I believe you need to treat master as if it's deployed to production. So once
>an API change is released, 'fixing' it needs to be done like any other API
>change - with a microversion bump and appropriate backwards compat.
>>
>> (For instance, I have a CI/CD pipeline merging from master every hour and
>doing a deploy - so 48 hours is a long time ago)
>>
>> Monty
>
>So let me jump in here and add in that a direct revert only should happen in
>extreme circumstances: aka a change that breaks behavior without a micro
>version bump - or something that is causing a break that cannot be fixed easily
>rolling forward. (Unable to land code in the gate at all for example, including
>roll forward fixes)
>
>In general (and especially with microversions) fail and fix moving forward is
>much better for the end users/deployers especially since folks are doing CD
>more aggressively now.
>
>There are other considerations but a revert really is one of the most extreme
>responses and should be used sparingly.


Just want to +1 the above. Master should be considered as deployed and we
shouldn't assume things. So, I'd advice a proper fix that is also backwards
compatible.

*cough* Doing a change on the API and then a revert feels like releasing on pypi
and then deleting the release. *cough*

Cheers,
Flavio

-- 
@flaper87
Flavio Percoco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20160304/ccfc7b6f/attachment.pgp>


More information about the OpenStack-dev mailing list