Re: [PATCH] Add test-string-list.c

Hi Thiago,

Thiago Farina wrote:

Add a simple test that demonstrates how to create and manipulate
a list of strings using the string-list.h API.

Quick thoughts:

Git programs tend to start with 

 #include "cache.h"

or

 #include "git-compat-util.h"

to get all the portability niceties.

+	print_string_list(&list, "");
+
+        int has_foo = string_list_has_string(&list, "foo");

Whitespace, declaration after statement... (see
Documentation/CodingGuidelines).

+	if (has_foo != 1)
+		error("List doesn't have foo.");

This does not exit with nonzero status when it fails.  You probably
wanted

	if (bad things)
		return error("problems!");

+	string_list_clear(&list, 0);
+	if (list.nr > 0)
+		error("List is not clear.");

To make sure this example remains valid, wouldn't you want to include
a caller in the t/ directory so it can be automatically run?  (See
t/README.)

Thoughts separate from the code:

 * it is probably worth mentioning Documentation/technical/api-string-list.txt
   for people who do not know about it.

 * for this to be useful as a test I think one has to sort of believe
   that it can break.  That is, a test of something this basic (which
   is already demonstrated and exercised by code throughout git, after
   all) would tend to be especially devious.

 * api-string-list.txt does not mention the STRING_LIST_INIT macros
   you introduced.  Maybe that would be worth improving.

Regards,
Jonathan