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(a)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/