Re: [PATCH] Disown ssh+git and git+ssh
- From
Carlos Martín Nieto <cmn@dwim.me>
- Date
- Mar 24, 2016, 16:56 UTC
- Message-ID
- <1458838567.179868.6.camel@dwim.me>
- In-Reply-To
- <xmqqbn6ngvs8.fsf@gitster.mtv.corp.google.com>
On Wed, 2016-03-09 at 13:56 -0800, Junio C Hamano wrote:
Show 28 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> >
> > It might be helpful to cite some reference to support the claim
> > that
> > they are "silly" since it's not necessarily obvious to readers who
> > did
> > not following the discussion.
> > ...
> > >
> > > || starts_with(url, "ssh://")
> > > + /*
> > > + * These ways to spell the ssh transport remain
> > > supported for
> > > + * compat but are undocumented and their use is
> > > discouraged
> > > + */
> > > || starts_with(url, "git+ssh://")
> > > || starts_with(url, "ssh+git://")) {
> > A little "comment" bikeshedding: Aside from undesirably
> > interrupting
> > the code flow, these large comment blocks draw far too much
> > attention
> > from the reader than these deprecated spellings of "ssh" deserve,
> > thus
> > making them seem overly important.
> I've been waiting for an update for it but got tired of it.
> Instead of discarding the topic, let's amend it like so:Sorry, I missed the call for the rewording. The below looks good to me. Thanks.
Show 77 quoted lines
>
> -- >8 --
> From: Carlos Martín Nieto <cmn@dwim.me>
> Date: Mon, 15 Feb 2016 15:29:06 +0100
> Subject: [PATCH] Disown ssh+git and git+ssh
>
> Some people argue that these were silly from the beginning (see
> http://thread.gmane.org/gmane.comp.version-control.git/285590/focus=2
> 85601
> for example), but we have to support them for compatibility.
>
> That doesn't mean we have to show them in the documentation. These
> were already left out of the main list, but a reference in the main
> manpage was left, so remove that.
>
> Also add a note to discourage their use if anybody goes looking for
> them
> in the source code.
>
> Signed-off-by: Carlos Martín Nieto <cmn@dwim.me>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/git.txt | 2 +-
> connect.c | 4 ++--
> transport.c | 5 +++--
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index d987ad2..2f90635 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -1122,7 +1122,7 @@ of clones and fetches.
> connection (or proxy, if configured)
>
> - `ssh`: git over ssh (including `host:path` syntax,
> - `git+ssh://`, etc).
> + `ssh://`, etc).
>
> - `rsync`: git over rsync
>
> diff --git a/connect.c b/connect.c
> index fd7ffe1..3babb81 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -267,9 +267,9 @@ static enum protocol get_protocol(const char
> *name)
> return PROTO_SSH;
> if (!strcmp(name, "git"))
> return PROTO_GIT;
> - if (!strcmp(name, "git+ssh"))
> + if (!strcmp(name, "git+ssh")) /* deprecated - do not use */
> return PROTO_SSH;
> - if (!strcmp(name, "ssh+git"))
> + if (!strcmp(name, "ssh+git")) /* deprecated - do not use */
> return PROTO_SSH;
> if (!strcmp(name, "file"))
> return PROTO_FILE;
> diff --git a/transport.c b/transport.c
> index 67f3666..908e08b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1001,8 +1001,9 @@ struct transport *transport_get(struct remote
> *remote, const char *url)
> || starts_with(url, "file://")
> || starts_with(url, "git://")
> || starts_with(url, "ssh://")
> - || starts_with(url, "git+ssh://")
> - || starts_with(url, "ssh+git://")) {
> + || starts_with(url, "git+ssh://") /* deprecated - do
> not use */
> + || starts_with(url, "ssh+git://") /* deprecated - do
> not use */
> + ) {
> /*
> * These are builtin smart transports; "allowed"
> transports
> * will be checked individually in git_connect.