[openstack-dev] [nova] Proposal new hacking rules
pcrews
gleebix at gmail.com
Mon Nov 24 18:02:58 UTC 2014
On 11/24/2014 09:40 AM, Ben Nemec wrote:
> On 11/24/2014 08:50 AM, Matthew Gilliard wrote:
>> 1/ assertFalse() vs assertEqual(x, False) - these are semantically
>> different because of python's notion of truthiness, so I don't think
>> we ought to make this a rule.
>>
>> 2/ expected/actual - incorrect failure messages have cost me more time
>> than I should admit to. I don't see any reason not to try to improve
>> in this area, even if it's difficult to automate.
>
> Personally I'd rather kill the expected, actual ordering and just have
> first, second or something that doesn't imply which value is which.
> Because it can't be automatically enforced, we'll _never_ fix all of the
> expected, actual mistakes (and will continually introduce new ones), so
> I'd prefer to eliminate the confusion by not requiring a specific ordering.
++. It should be a part of review to ensure that the test (including
error messages) makes sense. Simply having a (seemingly costly to
implement and enforce) rule stating that something must adhere to a
pattern does not guarantee that.
assertEqual(expected, actual, msg="nom nom nom cookie cookie yum")
matches the pattern, but the message still doesn't necessarily provide
much worth.
Focusing on making tests informative and clear about what is thought to
be broken on failure seems to be the better target (imo).
>
> Alternatively I suppose we could require kwargs for expected and actual
> in assertEqual. That would at least make it more obvious when someone
> has gotten it backward, but again that's a ton of code churn for minimal
> gain IMHO.
>
>>
>> 3/ warn{ing} - https://github.com/openstack/nova/blob/master/nova/hacking/checks.py#L322
>>
>> On the overarching point: There is no way to get started with
>> OpenStack, other than starting small. My first ever patch (a tidy-up)
>> was rejected for being trivial, and that was confusing and
>> disheartening. Nova has a lot on its plate, sure, and plenty of
>> pending code reviews. But there is also a lot of inconsistency and
>> unloved code which *is* worth fixing, because a tidy codebase is a joy
>> to work with, *and* these changes are ideal to bring new reviewers and
>> developers into the project.
>>
>> Linus' post on this from the LKML is almost a decade old (!) but worth reading.
>> https://lkml.org/lkml/2004/12/20/255
>>
>> MG
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
More information about the OpenStack-dev
mailing list