Re: [PATCH 0/2] Make plugin version checking more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 29 Mar 2018, at 09:14, Christophe de Dinechin <cdupontd@xxxxxxxxxx> wrote:
> 
> 
> 
>> On 28 Mar 2018, at 18:46, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
>> 
>> On Wed, Mar 28, 2018 at 06:06:19PM +0200, Christophe de Dinechin wrote:
>>>> If my task is to "move version check to the agent", do I _have_ to change
>>>> the semantics of the version check? No.
>>> 
>>> Of course you have to. There is no “PluginVersionIsCompatible”
>>> anymore, etc, so the version number semantics have changed whether you
>>> like it or not. You may artificially try to make the new version
>>> number look like the old one, and I would have if there wasn’t another
>>> problem with that numbering.
>> 
>> Yes, "another problem", which is why it's much better if we split them...
>> https://www.berrange.com/posts/2012/06/27/thoughts-on-improving-openstack-git-commit-practicehistory/
> 
> Which I will quote, then:
> 
> 	• Mixing two unrelated functional changes. Again the reviewer will find it harder to identify flaws if two unrelated changes are mixed together. If it becomes necessary to later revert a broken commit the two unrelated changes will need to be untangled, with further risk of bug creation.
> 
> I underline “unrelated”. I have proven that the changes were unrelated, and so did your own attempt at splitting, which require confusing and/or bug-introducing changes to the same piece of code.

*not* unrelated, obviously…

> 
>> 
>> This also makes the review process more complicated, as one has to
>> figure out what part of the patch is meant to achieve what. In this
>> case, I'd be fine ACK'ing the first 2 changes, but I haven't given much
>> thought regarding the versioning yet.
> 
> Maybe you should give it some thought then, instead of immediately jumping to conclusions and demanding that the patch be split.
> 
> 
> Thanks
> Christophe
> 

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]