From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Jeff King <peff@peff.net>, Marko Kungla <marko.kungla@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] check-ref-format: require a repository for --branch
Date: Tue, 17 Oct 2017 16:05:43 +0900 [thread overview]
Message-ID: <xmqqinfentiw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171017044111.ahe4eiepvokecnkr@aiede.mtv.corp.google.com> (Jonathan Nieder's message of "Mon, 16 Oct 2017 21:41:11 -0700")
Jonathan Nieder <jrnieder@gmail.com> writes:
> And in that spirit, I think the patch you replied with aims to go in
> the right direction, by providing the core functionality when in a
> repository while avoiding breaking such a script outside of one
> (though I do not understand it fully yet).
Given that, is it safe for me to ignore this earlier one
> For what it's worth, I don't agree with this repurposing of
> "check-ref-format --branch" at all.
as reacting to the patch without reading what it does?
>> (e.g. a wrapper to "git
>> clone" that wants to verify the name it is going to give to the "-b"
>> option), and a check desired in such a context is different from
>> (and is stricter than) feeding refs/heads/$name to the same command
>> without the "--branch" option.
>
> Can you say more about this example? E.g. why is this hypothetical
> wrapper unable to rely on "git clone -b"'s own error handling?
If I have to guess what you meant, perhaps the Porcelain wanted to
diagnose bad parameter that would be rejected _before_ letting clone
spend cycles and possibly network bandwidth?
But this was my way of rephrasing an earlier example you used while
objecting to Peff's change
For example, if you have a script with an $opt_branch variable
that defaults to "master", it may do
resolved_branch=$(git check-ref-format --branch "$opt_branch")
even though it is in a mode that not going to have to use
$resolved_branch and it is not running from a repository.
so my answer to the question in the message I am directly responding
to would be "You tell me." ;-)
> --symbolic-full-name seems like a good fit. Do you remember why
> check-ref-format was introduced instead?
It really serves two purposes, and at this point, I do not think it
really matters why it also does the @{-1} expansion as well as name
validation. 604e0cb5 ("Documentation: describe check-ref-format
--branch", 2009-10-12) happened 8 years ago, and since then it has
been advertised long enough as if the 80% primary purpose of "c-r-f
--branch" were to expand @{-1} to a branch name; even though the
text hints that it also does the usual checks, by definition
validation of the result of expanding @{-1} ought to succeed; after
all that was the branch we _were_ on ;-).
next prev parent reply other threads:[~2017-10-17 7:05 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-08 1:17 bug with check-ref-format outside of repository Marko Kungla
2017-07-14 18:03 ` Jeff King
2017-07-14 18:18 ` [PATCH] check-ref-format: require a repository for --branch Jeff King
2017-07-17 17:27 ` Jonathan Nieder
2017-07-17 21:18 ` Marko Kungla
2017-08-17 10:23 ` Jeff King
2017-08-17 10:22 ` Jeff King
2017-08-17 21:30 ` Junio C Hamano
2017-08-18 6:20 ` Jeff King
2017-08-18 7:45 ` Junio C Hamano
2017-10-16 6:44 ` Junio C Hamano
2017-10-16 10:45 ` Junio C Hamano
2017-10-16 22:45 ` Jeff King
2017-10-16 23:00 ` Jonathan Nieder
2017-10-17 1:22 ` Junio C Hamano
2017-10-17 2:42 ` Jeff King
2017-10-17 3:33 ` Junio C Hamano
2017-10-17 4:44 ` Junio C Hamano
2017-10-17 7:06 ` [PATCH 0/3] " Jonathan Nieder
2017-10-17 7:08 ` [PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository Jonathan Nieder
2017-10-17 7:10 ` [PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix Jonathan Nieder
2017-10-17 7:12 ` [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch Junio C Hamano
2017-10-17 7:17 ` Jonathan Nieder
2017-10-17 8:21 ` Junio C Hamano
2017-10-17 7:12 ` [PATCH 3/3] check-ref-format doc: --branch validates and expands <branch> Jonathan Nieder
2017-10-17 20:55 ` Junio C Hamano
2017-10-17 21:06 ` Jonathan Nieder
2017-10-18 5:10 ` Jeff King
2017-10-17 1:27 ` [PATCH] check-ref-format: require a repository for --branch Kevin Daudt
2017-10-17 2:40 ` Junio C Hamano
2017-10-17 4:30 ` Kevin Daudt
2017-10-16 22:42 ` Jeff King
2017-10-17 4:41 ` Jonathan Nieder
2017-10-17 7:05 ` Junio C Hamano [this message]
2017-10-17 7:25 ` Jonathan Nieder
2017-10-17 7:34 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqinfentiw.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=marko.kungla@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.