On 07/20/2011 12:58 PM, Jan Zelený wrote: > I'm sending several patches with the sysdb refactoring. Originally, I was > planning one more step - to change return type from int to errno_t where it > was convenient. If someone wants to do it, feel free. Otherwise I'm going to > make the patch myself in couple days. > > When reviewing, please note that patches 40-43 have to be tested together. > The > same applies for patches 44-46. > > Jan >
039 - what do you think about renaming "sysdb_ctx_list *ctx_list" as
well? Otherwise ACK.
I think the way the patches are split is wrong -- after you apply 40,
you can't even *compile* the code. If we ever wanted to cherry-pick any
of the changes to some other branch, it would be all-or-nothing.
I think the split should either be per-function, or, if you think that's
too many patches perhaps per group of functions ("all functions that
handle users", "all functions that handle groups" etc.).
We already did a huge sysdb API rewrite once, in 1.3, and the patches
were per-function back then -- check out
d86fc9163127f7c5bd0c3af950fcddff7911867f and earlier.
Considering that it may be a lot of work to regroup the patches, I think
it may be even better to have squash 40-43 and 44-46 together, but a
patch that does not compile seems bad to me.
After applying the patches (there were two minor conflicts), "make
check" and a simple smoke test passed. I didn't review the patches
completely yet.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
