-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/18/2013 09:17 AM, Abhishek Singh wrote: > Hi Jackub, > > I have attached a patch. Kindly check it. > > thanks, >
An excellent start, Abhishek! I have a few comments (mostly style): Please make sure to maintain the same whitespace that was used in the original code you modify. For example, you added tabs in the HAVE_CMOCKA section of Makefile.am where the other code is indented with spaces. Also, you have trailing whitespace after --coverage in the _CFLAGS section, as well as two lines in test_check_if_uid_is_active_fail() that consist of nothing but whitespace. Many of us use editors that warn of such things with red overlays. If you're using VIM, I can recommend a .vimrc setting that will display them to you as well. Also, in main(), when you're creating the list of tests, it would be more readable in the form: const UnitTest tests[] = { unit_test(test_check_if_uid_is_active_success), unit_test(test_check_if_uid_is_active_fail), unit_test(test_get_uid_table) }; When I first glanced at it, I didn't notice the _fail test because it was on the same line as the _success test. All tests pass when run and it builds without warnings. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlFHF28ACgkQeiVVYja6o6MSsQCgm6rpXpiKTCrf/Y4JYLQfClTY kKkAnRdupgXGNusltLWZkVII2nVaOvxk =ZKm1 -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel