[openstack-dev] [trove][neutron][cinder][swift][ceilometer][nova][keystone][sahara][glance][neutron-lbaas][imm] stylistic changes to code, how do we handle them?
ihrachys at redhat.com
Tue Jan 12 14:01:46 UTC 2016
Amrith Kumar <amrith at tesora.com> wrote:
> I've tagged this message with the projects impacted by a series of change
> [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 ...
> 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.
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.
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
More information about the OpenStack-dev