Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits
- Date: Tue, 10 Jul 2018 23:18:22 -0700
- From: William Chargin <wchargin@xxxxxxxxx>
- Subject: Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits
> we care about reachability only from the detached HEAD here, so this
> must _not_ use test_commit, which creates an extra tag.
Right. I can add a comment to that effect.
> Please avoid unnecessary reflowing of earlier lines of the paragrpah
> when the only change is to insert "or from HEAD" in the middle of its
> fourth line.
Sure: I'll make that change. (My intent was to incrementally clean up an
area already under change, but I'm happy to instead make only the
> Also, I am not sure if "or from HEAD" is even needed when we say
> "from ANY ref" already, as we count things like HEAD as part of the
> ref namespace.
My two cents: with the docs as is, I wasn't sure whether HEAD was
intended to count as a ref for this purpose. The gitglossary man page
defines a ref as a "name that begins with refs/" (seemingly excluding
HEAD), though it later says that HEAD is a "special-purpose ref". In my
opinion, the change adds clarity without any particular downside---but
I'm happy to revert it if you'd prefer. I'd also be happy to change the
wording to something like "any ref, including HEAD" if we want to
emphasize that HEAD really is a ref.
> We are interested in seeing that ":/string" is understood as a valid
> object name, and this is not limited to "log" at all.
Indeed. I was a bit surprised for the same reason (I expected the tests
to be using `rev-parse`), but I agree that it's probably best to keep
the existing structure to minimize churn.
After reaching consensus on the change to the docs, should I send in a
[PATCH v2] In-Reply-To this thread? Peff, should I add your
Signed-off-by to the commit message, or is that not how things are done?