[SSSD] [PATCHES] Sysdb refactoring

Jan Zelený jzeleny at redhat.com
Fri Jul 29 06:39:03 UTC 2011


> 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.

The only reason to do it this way was to separate the change in sysdb itself 
from the rest of code changes - mainly for the sake of lucidity.

> 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.

This sounds reasonable. Should I do the job and send them back or were you 
thinking about squashing them during the push?

> 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.

Thanks for doing the review
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110729/8e410524/attachment.sig>


More information about the sssd-devel mailing list