[SSSD] [PATCHES] INI meta data interface patches
Dmitri Pal
dpal at redhat.com
Mon Apr 12 12:31:47 UTC 2010
Jakub Hrozek wrote:
> On 04/09/2010 01:31 AM, Dmitri Pal wrote:
> > I think I addressed all the issues except trace.
>
> Thank you! I noticed two more issues:
>
> * in check_id, instead of "if ((error) || (id == -1))" you probably
> meant "|| (fid == -1)"
> * in ini_metadata.c, the constant #define CHEKED_ALL 3 is not used
> anywhere (and the name contains a typo), it looks like something that
> was meant to be used in config_changed()
>
I will look into those.
> > Set of new patches attached.
> > I noticed that I added "can_free" back again in patch 3. I fixed it in
> > patch 4.
> > Addressing issues in 4 patches is not a simple task I should say...
>
> git rebase -i usually works for me..minus an occasional conflict when
> squashing a fix into a commit deep down in the tree.
>
> I have also one more general comment..the way these patches are
> structured is not ideal w.r.t patch review and management - patch 0002
> adds some code that uses col_add_int_property() and then patch 0004
> changes that into using col_add_str_property. It doesn't matter when
> reading the resulting code but I think it would be easier for anyone
> reading the patch series if there was only one addition with the
> str_property only.
This is because I made a mistake in using the col_add_int_property in
the first place.
And realized it finishing the fourth patch.
_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
More information about the sssd-devel
mailing list