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:[email protected]]
> Sent: Tuesday, January 12, 2016 9:03 AM
> To: OpenStack Development Mailing List (not for usage questions)
> <[email protected]>
> 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
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev