On Wed, 2009-08-26 at 11:01 +0200, Jakub Hrozek wrote:
> One more change, while working on #138 I realized that the copyright
> headers were incorrect -- I had a GPLv2 header in my personal c source
> file template, while the rest of the project uses GPLv3.
> 
> I also squashed in a fix for a typo in user-visible error message
> (fixes
> #136) into patch #1.

I have given some more detail feedback on IRC, but here for the records.

Patch 1:
- mostly good but mem hierarchy between ops_ctx and tools_ctx needs to
be reversed
- also some cases where codyng style is not followed (missing space
between 'if' and '('

Patch 2: NACK
- do not create tools/common/ keep all in tools/ until we have a lot's
of files to get out of the way
- main NACK point is that tevent_req async coding style has not been
followed in the sync wrappers, details explained on IRC
- also do the transaction as a separate operation and simply pass in the
handle to the sync ones, so that multiple sync operations can be linked
into a single transaction.

Patch 3:
- looks sane but I'd like a second look from one of ours python resident
experts

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to