On Mon, May 5, 2014 at 1:56 PM, Ben Nemec <openst...@nemebean.com> wrote:
> On 05/05/2014 10:02 AM, ChangBo Guo wrote:
>>
>> Hi Stackers,
>>
>> I find some common code style would be avoided while I'm reviewing code
>> ,so think these check would
>> be nice to move into local hacking. The local hacking can ease reviewer
>> burden.
>>
>> The idea from keystone blueprint [1].
>> Hacking is a great start at automating checks for common style issues.
>> There are still lots of things that it is not checking for that it
>> probably should. The local hacking ease reviewer burden . This is the
>> list of from [1][2] that would be nice to move into an automated check:
>>
>> - use import style 'from openstack.common.* import' not use 'import
>> openstack.common.*'
>
>
> This is the only one that I think belongs in Oslo. The others are all
> generally applicable, but the other projects aren't going to want to enforce
> the import style since it's only to make Oslo syncs work right.
+1, and only for the incubator repository
>> - assertIsNone should be used when using None with assertEqual
>> - _() should not be used in debug log statements
>> -do not use 'assertTrue(isinstance(a, b)) sentence'
>> -do not use 'assertEqual(type(A), B) sentence'
>
>
> The _() one in particular I think we'll want as we make the logging changes.
> Some additional checks to make sure the the correct _ function is used with
> the correct logging function would be good too (for example,
> LOG.warning(_LE('foo')) should fail pep8).
>
> But again, that belongs in hacking proper, not an Oslo module.
+1
> The assert ones do seem to fit the best practices as I understand them, but
> I suspect there's going to be quite a bit of work to get projects compliant.
I've seen some work being done on that already, but I don't know how
strongly we care about those specific rules as an overall project.
Doug
>
>>
>> [1]
>> https://blueprints.launchpad.net/keystone/+spec/more-code-style-automation
>> [2] https://github.com/openstack/nova/blob/master/nova/hacking/checks.py
>>
>> I just registered a blueprint for this in [3] and submit first patch in
>> [4].
>>
>> [3] https://blueprints.launchpad.net/oslo/+spec/oslo-local-hacking
>>
>> [4] https://review.openstack.org/#/c/87832/
>> <https://github.com/openstack/nova/blob/master/nova/hacking/checks.py>
>>
>>
>> Should we add local hacking for oslo-incubator ? If yes, what's the
>> other check will be added ?
>> Your comment is appreciated :-)
>>
>> --
>> ChangBo Guo(gcb)
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
