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:
> +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).
-Peff
--
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
