[SSSD] [PATCHSET] intg: sss_override

Pavel Reichl preichl at redhat.com
Mon Oct 12 21:22:38 UTC 2015



On 10/07/2015 06:35 PM, Nikolai Kondrashov wrote:
> On 10/07/2015 07:10 PM, Pavel Reichl wrote:
>>> We can use either mine (after modifications) or yours.
>>
>> I suppose yours are closer to be acked so we should probably use yours.
>
> I'm not so sure, but alright :)
>
>> All valid comments, but I will ignore them for now as I suppose this patch
>> will be superseded by yours.
>
> Alright.
>
>>> Just checking a string for truth wouldn't be sufficient, because an empty
>>> string is False and we might need to pass it sometimes. So, I think, we need
>>> to test against None here and above.
>>
>> Thanks, my python skills got a bit rusty I suppose :-)
>
> Mine were never sharp, but I'm generally vary of a language's concept of
> truth (seen many strange ones), so I checked :)
>
>>> I really don't like how we keep copying this stuff around. I'll look into
>>> making a module with these.
>>
>> OK, should I postpone amending the patch until your patch is in master or
>> should I do it as part of this patch set?
>
> I think it will be more efficient for you to keep working on your patch as is,
> in this part, and if I manage to produce something, then we can resync.
>
>>> Could you please add descriptions for all functions, especially these two?
>>
>> Sure. I wasn't sure about those functions. Intention was to verify that
>> state of overrides is the same after sequence of commands - for example just
>> before we call 'sss_override user-export' and then after we call
>> 'sss_override user-import'. I wanted to avoid copy&paste approach. But
>> wasn't sure how to find out a descriptive name. Would something like
>> check_user_override_state_1() be better name?
>
> I can't really tell, since I haven't dug into what the tests actually do yet.
> So far I'm just reacting to somewhat generically-named functions without
> descriptions. I'm reserving the functional review until it's easier :)
>
> Thanks for taking my comments into account :)!
>
> Nick
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Please see updated patchset.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-intg-new-test-for-user-local-overrides.patch
Type: text/x-patch
Size: 11377 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151012/7a6384f6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-intg-new-test-for-group-local-overrides.patch
Type: text/x-patch
Size: 4710 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151012/7a6384f6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-intg-local-overrides-regression-tests.patch
Type: text/x-patch
Size: 4799 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151012/7a6384f6/attachment-0002.bin>


More information about the sssd-devel mailing list