Open Stack

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 
>>change sets:
>>
>>	[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 ...
>>
>>To
>>
>>if var:
>>	... 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.
>
>* https://review.openstack.org/#/c/266156/1/neutron/agent/linux/daemon.py
>
>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 
>here.
>
>>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.
>>
>>Thanks,
>>
>>-amrith
>>
>>
>>__________________________________________________________________________
>>OpenStack Development Mailing List (not for usage questions)
>>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
>__________________________________________________________________________
>OpenStack Development Mailing List (not for usage questions)
>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-- 
@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/20160112/2c7022c2/attachment.pgp>