<div dir="ltr"><div>Changes done.<br><br></div><div>My public repo of sssd : <a href="https://github.com/AbhishekKumarSingh/sssd">https://github.com/AbhishekKumarSingh/sssd</a><br></div></div><div class="gmail_extra"><br><br>
<div class="gmail_quote">On Mon, Mar 18, 2013 at 7:02 PM, Stephen Gallagher <span dir="ltr">&lt;<a href="mailto:sgallagh@redhat.com" target="_blank">sgallagh@redhat.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<div class="im"><br>
On 03/18/2013 09:17 AM, Abhishek Singh wrote:<br>
&gt; Hi Jackub,<br>
&gt;<br>
</div><div class="im">&gt; I have attached a patch. Kindly check it.<br>
&gt;<br>
&gt; thanks,<br>
&gt;<br>
<br>
<br>
</div>An excellent start, Abhishek!<br>
<br>
I have a few comments (mostly style):<br>
<br>
Please make sure to maintain the same whitespace that was used in the<br>
original code you modify. For example, you added tabs in the<br>
HAVE_CMOCKA section of Makefile.am where the other code is indented<br>
with spaces. Also, you have trailing whitespace after --coverage in<br>
the _CFLAGS section, as well as two lines in<br>
test_check_if_uid_is_active_fail() that consist of nothing but<br>
whitespace. Many of us use editors that warn of such things with red<br>
overlays. If you&#39;re using VIM, I can recommend a .vimrc setting that<br>
will display them to you as well.<br>
<br>
<br>
Also, in main(), when you&#39;re creating the list of tests, it would be<br>
more readable in the form:<br>
<br>
const UnitTest tests[] = {<br>
    unit_test(test_check_if_uid_is_active_success),<br>
    unit_test(test_check_if_uid_is_active_fail),<br>
    unit_test(test_get_uid_table)<br>
};<br>
<br>
When I first glanced at it, I didn&#39;t notice the _fail test because it<br>
was on the same line as the _success test.<br>
<br>
<br>
All tests pass when run and it builds without warnings.<br>
-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v1.4.13 (GNU/Linux)<br>
Comment: Using GnuPG with Thunderbird - <a href="http://www.enigmail.net/" target="_blank">http://www.enigmail.net/</a><br>
<br>
iEYEARECAAYFAlFHF28ACgkQeiVVYja6o6MSsQCgm6rpXpiKTCrf/Y4JYLQfClTY<br>
kKkAnRdupgXGNusltLWZkVII2nVaOvxk<br>
=ZKm1<br>
-----END PGP SIGNATURE-----<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
sssd-devel mailing list<br>
<a href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a><br>
<a href="https://lists.fedorahosted.org/mailman/listinfo/sssd-devel" target="_blank">https://lists.fedorahosted.org/mailman/listinfo/sssd-devel</a><br>
</div></div></blockquote></div><br></div>