On Tue, Aug 26, 2014 at 02:01:07PM +0200, Pavel Reichl wrote:
> On 08/26/2014 11:37 AM, Jakub Hrozek wrote:
>> On Mon, Aug 25, 2014 at 08:18:15PM +0200, Pavel Reichl wrote:
>>> On 08/25/2014 06:21 PM, Jakub Hrozek wrote:
>>>> On Mon, Aug 25, 2014 at 05:23:44PM +0200, Pavel Reichl wrote:
>>>>> Hello,
>>>>>
>>>>> I tested the patch and it fixes the problem described in the ticket.
>>>>>
>>>>> I personally dislike the break and mainly the continue check - I
believe
>>>>> that people reading the code would benefit from encapsulating the
inner loop
>>>>> into function (return value would indicate what is happening and
function
>>>>> name is the best comment anyway) and same would go for the outer loop
IMO.
>>>> That's a fair comment. Would you prefer to split the for loop into
one
>>>> function or two?
>>> two, but I can live with one :-)
>> OK, see the attached patches. Please note I wasn't able to test them
>> properly because my new AD test servers don't carry POSIX attributes
>> yet. I'll test the patches in a short while, in the meantime, feel free
>> to test in your environment and/or look at the code.
>>
> After applying the patches I can't replicate the problem any longer.
>
> Thanks for the code style change I think it's better now.
>
> Just 2 little details, please see inline, but I'm willing to ACK patches as
> are now.
I fixed both. Thanks for the review.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel