> 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.Ok, will do Jan
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
