[Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin

Petr Vobornik pvoborni at redhat.com
Thu May 28 08:26:03 UTC 2015


On 05/27/2015 08:17 PM, Martin Basti wrote:
> On 27/05/15 19:27, Rob Crittenden wrote:
>> Martin Basti wrote:
>>>>
>>> Thank you.
>>>
>>> I haven't finished review yet, but I have few notes in case you will
>>> modify the patch.
>>>
>>> Please fix following issues:
>>>
>>
>>> 3)
>>> There are many PEP8 errors, can you fix some of them,?
>>
>> Is PEP8 a concern? What kinds of errors do we fix? For example, the
>> current model for defining options generates a slew of indention errors.

In old modules it's preferred to keep the old indentation style for 
options(not to mix 2 styles). New modules should use following pep8 
compliant style:
         Str(
             'cn',
             cli_name='name',
             primary_key=True,
             label=_('Server name'),
             doc=_('IPA server hostname'),
         ),

> We try to keep PEP8 in new code, mainly indentation, blank lines, too
> long lines.
> Yes in test definitions and option definitions, is better to keep the
> same style, but other parts of code should be PEP8.
>
> For example these should be fixed
> ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
> missing whitespace around operator
> ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
> expected 2 blank lines, found 1
> ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
> expected 2 blank lines, found 1
>
>>
>>
>> I'll wait and see what falls out of the API review before making any
>> real changes.
>>
>> rob
>>
>
> Martin^2
>


-- 
Petr Vobornik




More information about the Freeipa-devel mailing list