[SSSD] [PATCHES] Sysdb refactoring

Stephen Gallagher sgallagh at redhat.com
Fri Jul 29 12:42:57 UTC 2011


On Fri, 2011-07-29 at 08:39 +0200, Jan Zelený wrote:
> > 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


Just for the record, I'm intentionally deferring this commit until we
branch sssd-1-6, because it's too sizable a change to make this close to
release.

Jan, please keep an eye on these patches and make sure they continue to
apply against master. It will be the first set to go in once we branch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110729/1d6916e8/attachment.sig>


More information about the sssd-devel mailing list