Amrith Kumar <amr...@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-requ...@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-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
