Re: [PATCH] Disown ssh+git and git+ssh

["Carlos Martín Nieto" <cmn@dwim.me>]

On Wed, 2016-03-09 at 13:56 -0800, Junio C Hamano wrote:
> 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.

> 
> -- >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.