[openstack-dev] [trove][neutron][cinder][swift][ceilometer][nova][keystone][sahara][glance][neutron-lbaas][imm] stylistic changes to code, how do we handle them?
flavio at redhat.com
Tue Jan 12 14:35:43 UTC 2016
On 12/01/16 15:01 +0100, Ihar Hrachyshka wrote:
>Amrith Kumar <amrith at tesora.com> wrote:
>>I've tagged this message with the projects impacted by a series of
>> [trove] https://review.openstack.org/#/c/266220/
>> [neutron] https://review.openstack.org/#/c/266156/1
>> [cinder] https://review.openstack.org/#/c/266099/2
>> [swift] https://review.openstack.org/#/c/266185/1
>> [ceilometer] https://review.openstack.org/#/c/266211/1
>> [nova] https://review.openstack.org/#/c/266143/2
>> [keystone] https://review.openstack.org/#/c/266203/2
>> [sahara] https://review.openstack.org/#/c/266230/1
>> [glance] https://review.openstack.org/#/c/266192/1
>> [neutron-lbaas] https://review.openstack.org/#/c/266181/1
>>I would like the guidance of the developer community in figuring out
>>how to proceed with this change, and changes like this.
>>The change, in essence changes a construct of the kind:
>>if var > 0:
>> ... something ...
>> ... something …
>The change is not stylistic. F.e. before the change, var == -1 would
>not result in triggering the conditional body; while in the new
>version, it does trigger it. Unless the assumption that var is >= 0 is
>always true, it’s just a wrong change. F.e. I suspect in *, you
>effectively make error code returned by fork() (-1) to be considered
>as successful value.
>I also don’t see any value in such a change.
In addition to the above, I believe `if var > 0` is faster than `if var` just
like `if x is None` is (normally?) faster than `if not x`.
>>In a couple of cases, it also changes messages from (for example, in Trove)
>>"Limit value must be > 0" to
>>"Limit value must be greater than zero"
>>My question to the ML is this, should stylistic changes of this kind
>>be handled in a consistent way across all projects, maybe with a
>>hacking rule and some discussion on the ML first? After all, if this
>>change is worthwhile, it is worth ensuring that this construct that
>>we are seeking to eliminate, does not reenter the code base.
>If a stylistic change is worth it and can be automated, yes, it’s
>better to have a hacking rule for that. Though I doubt that’s the case
>>For what it is worth, I agree with Vitaly Grindev [sahara, in review
>>https://review.openstack.org/#/c/266230/1]. I think the code before
>>the change was more intuitive and readable.
>>OpenStack Development Mailing List (not for usage questions)
>>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>OpenStack Development Mailing List (not for usage questions)
>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 819 bytes
Desc: not available
More information about the OpenStack-dev