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.