On Sun, Jul 07, 2013 at 03:33:44PM -0700, Junio C Hamano wrote:
> With this on top of the other patches in this series, you would get:
>
> $ git describe --contains $(git rev-parse v1.8.3 v1.8.3^0)
> v1.8.3
> v1.8.3
>
> while you can still differentiate tags and the commits they point at
> with:
>
> $ git name-rev --refs=tags/\* --name-only $(git rev-parse v1.8.3 v1.8.3^0)
> v1.8.3
> v1.8.3^0
>
> The difference in these two behaviours is achieved by adding --peel-to-commit
> option to "name-rev" and using it when "describe" internally calls it.
I am somewhat mixed on this.
You are changing the default behavior of name-rev and adding a new
option to restore it, so I wonder who (if anyone) might be broken. The
documentation is now also out of date; not only does it not mention
"peel-to-commit", but it claims the argument to name-rev is a
committish, which is not really true without that option.
On the other hand, the new default behavior seems way more sane to me.
In general, I would expect name-rev to:
1. Behave more or less the same between "git name-rev $sha1" and "echo
$sha1 | git name-rev --stdin". Your patch improves that. Though I
note that --peel-to-commit does not affect --stdin at all. Should
it? And of course the two differ in that the command line will take
any rev-parse expression, and --stdin only looks for full sha1s.
2. If name-rev prints "$X $Y", I would expect "git rev-parse $X" to
equal "git rev-parse $Y". With peeling, that is not the case, and
you get the misleading example that Ram showed:
$ git name-rev 8af0605
8af0605 tags/v1.8.3^0
or more obviously weird:
$ git name-rev v1.8.3
v1.8.3 tags/v1.8.3^0
So I think your series moves in a good direction, but I would just worry
that it is breaking backwards compatibility (but like I said, I am not
clear on who is affected and what it means for them).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
