Open Stack

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 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