On Sun, Sep 5, 2010 at 2:02 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:>
>
> Git programs tend to start with
>
> #include "cache.h"
>
> or
>
> #include "git-compat-util.h"
>
> to get all the portability niceties.
>
Including it now.
>> + print_string_list(&list, "");
>> +
>> + int has_foo = string_list_has_string(&list, "foo");
>
> Whitespace, declaration after statement... (see
> Documentation/CodingGuidelines).
>
Fixed.
>> + 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!");
>
Fixed.
>> + 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.)
>
I read it, but I'm not sure how to do this. Maybe you could point me
to an example?
> Thoughts separate from the code:
>
> * 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.
>
It is basic, so anyone can read it, and say "Oh, I can do this.".
Looking through the code maybe not so easy.
It can be expanded later by anyone to test many other things though.
So, why not? (Is it so bad to not have it at all?).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html