-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/28/2009 05:49 PM, Simo Sorce wrote:
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 '('
Should be done. Note that the patch is different than the original one
b/c of the removal of shadow-utils fallback.
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
done
- main NACK point is that tevent_req async coding style has not been
followed in the sync wrappers, details explained on IRC
OK, I pretty much rewrote the whole thing to be tevent_req-stylish with
_send _recv etc. I agree that it's way more readable now.
- 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.
Transactions are now done outside the "sync" module, that is, either in
the tools or in the python bindings module.
Patch 3:
- looks sane but I'd like a second look from one of ours python resident
experts
CC-ed John.
Thanks for the review!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAkqpcVwACgkQHsardTLnvCUUTgCgyBwqkDnhrFb+Z4a6NHdM9h4k
zScAoNwiXi7byuGuXyGg6VeNX82KpMuR
=8NYQ
-----END PGP SIGNATURE-----