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

[Rob Crittenden]

Petr Vobornik wrote:
> 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

Updated API and addressed Martin's concerns. The regex must have been a 
bad copy/paste, it is fixed now.

The design page has been updated as well.

rob

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1112-4-Add-plugin-to-manage-service-constraint-delegations.patch
Type: text/x-diff
Size: 57212 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150530/9f3cb974/attachment.bin>