On Wed, Jul 09, 2014 at 03:36:51PM -0700, Jacob Keller wrote:

> Add support for configuring default sort ordering for git tags. Command
> line option will override this configured value, using the exact same
> syntax.

Thanks, this version looks pretty good to me. A few minor comments:

> +     if (!strcmp(var, "tag.sort")) {
> +             tag_sort = parse_sort_string(value);
> +     }

Our style is to usually avoid braces for a one-liner. However, I think
would actually make sense to "return 0" from this conditional.

> +test_expect_success 'configured lexical sort' '
> +     git config tag.sort "v:refname" &&
> +     git tag -l "foo*" >actual &&
> [...]

Please use:

  test_config tag.sort "v:refname"

here, which will clean up the config value after the test ends (and thus
not pollute any later tests).

Though you will need to add an extra "test_config" to the following

> +test_expect_success 'option override configured sort' '
> +     git tag -l --sort=-refname "foo*" >actual &&
> [...]

I think that's a good thing, though (it makes it more clear in the
second test what is being tested, rather than relying on the state left
by the previous test).

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to