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...@anticdent.org] > Sent: Tuesday, January 12, 2016 9:03 AM > To: OpenStack Development Mailing List (not for usage questions) > <openstack-dev@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
__________________________________________________________________________ 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