[openstack-dev] [trove][neutron][cinder][swift][ceilometer][nova][keystone][sahara][glance][neutron-lbaas][imm] stylistic changes to code, how do we handle them?

Amrith Kumar amrith at tesora.com
Tue Jan 12 14:13:55 UTC 2016


Chris, Ihar,

I assumed that this was stylistic based on the fact that in the places where I was seeing it, it seemed to be the case that the LHS was intuitively positive (a length, for example). I did not exhaustively verify this but yes, you are correct, the construct 

	If var 

is the same as  (I believe)

	if var is not None

and therefore the change does change the semantic of the code. Modulo my assumption though, it would be strictly stylistic which is why I phrased it as such.

Thanks for the quick responses, so far it sounds like the tally is:

Not-Worthwhile: 3
Changes semantics: 1
Others: 0

I created a quick poll to tally results

https://www.surveymonkey.com/r/DLRN9TP

Thanks,

-amrith




> -----Original Message-----
> From: Chris Dent [mailto:cdent+os at anticdent.org]
> Sent: Tuesday, January 12, 2016 9:03 AM
> To: OpenStack Development Mailing List (not for usage questions)
> <openstack-dev at lists.openstack.org>
> Subject: Re: [openstack-dev]
> [trove][neutron][cinder][swift][ceilometer][nova][keystone][sahara][glance
> ][neutron-lbaas][imm] stylistic changes to code, how do we handle them?
> 
> On Tue, 12 Jan 2016, Amrith Kumar wrote:
> 
> > if var > 0:
> > 	... something ...
> >
> > To
> >
> > if var:
> > 	... something ...
> 
> I may be missing something but the above is not a stylistic change if var can
> ever be negative. In one of the ceilometer changes[1] for example, this
> change will change the flow of the code. In this particular example if some
> caller do _do_test_iter_images sends
> page_size=-1 bad things will happen. Since it is test code the scope of the
> damage is limited, but I prefer the more explicit > 0.
> 
> I've not checked all the reviews but if it is showing up in one place seems like
> it could in others.
> 
> [1]
> https://review.openstack.org/#/c/266211/1/ceilometer/tests/unit/image/te
> st_glance.py
> 
> --
> Chris Dent               (¨s¡ã¡õ¡ã)¨s¦à©ß©¥©ß            http://anticdent.org/
> freenode: cdent                                         tw: @anticdent


More information about the OpenStack-dev mailing list