[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