-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/05/2009 01:08 PM, Stephen Gallagher wrote:
On 10/05/2009 12:55 PM, Dmitri Pal wrote:
> Jakub Hrozek wrote:
>
>> On 10/02/2009 06:49 PM, Dmitri Pal wrote:
>>
>>> Updated versions are attached.
>>> Comments are inline.
>>>
>>
>> ACK to all three
>>
>>
>>>>> 0001:
>>>>> * Is it possible to change the callbacks in elapi_resolve.c to
static?
>>>>>
>>> Done
>>>
>>
>> Just one thing, it is better to squash the changes to the file that
>> actually introduces the functions, that way you have them correct in the
>> first patch, rather than non-static in first patch and fixed to static
>> in second.
>>
>>
> I agree, but I think I had two sets of changes in the same file: one set
> for the first patch (changing to static) and another for the second (do
> not remember).
> To avoid doing editing and squashing twice I did it once and added on
> top of the second patch.
> Is it a bad practice? Seems like a bit of overkill to me to do it twice
> but I am a novice to the common practices of git so please guide me :-).
>
>
Yes, it's a bad practice to have unrelated changes in a patch. Most
notably because it makes it difficult to apply just one of them if the
other isn't ready yet.
There are a couple ways to split patches. The easiest way is probably
with 'git add -i' (interactive) which allows you to select individual
portions of a diff for including into a commit.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/sssd-devel
>
>
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Nack.
sss_getpwnam_done() and friends should return EIO if the sysdb replies
with more than one result, I think. EINVAL implies that the fault was
with the caller, whereas EIO shows us that something went wrong internally.
I don't like the gratuitous use of discard_const_p in
sss_getpwnam_done(). These sync ops are theoretically useful in other
places in our code. I don't want people assuming that they can modify
the contents of the res->data->name field (for example). They should be
talloc_strdup()-ed into the res structure.
sss_useradd.c:
ERROR("A user or group with the same name or UID already exists\n");
should be
ERROR("A user or group with the same name or ID already exists\n");
tools_util.c:
Shouldn't we just make a call to something like a sysdb_getgrent_sync
and compare the resulting complete list with the received groups instead
of making N complete sync calls, where N could be equal to (or greater
than) the total list of groups in the DB?
- --
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAkrKLhkACgkQeiVVYja6o6PzGQCdEJH9sI9XB9fahw5gwZyPKv3I
23oAn2u9MVzvzjzEQPn3HLP67nAic83Y
=FhY7
-----END PGP SIGNATURE-----