Re: [PATCH v3 4/4] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config

Hi Junio,

On Wed, 1 Feb 2017, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Johannes Schindelin [off-list ref] writes:

quoted

That leaves the "putty" case in handle_ssh_variant(), does it not? Was it
not your specific objection that that is the case?

Yup, you can remove that while you reroll.

quoted

No worries, I will let this simmer for a while. Your fixup has a lot of
duplicated code (so much for maintainability as an important goal... ;-))
and I will have to think about it. My immediate thinking is to *not*
duplicate code,...

You need to realize that the namespaces of the configuration and the
command names are distinct.  There is no code duplication.

To explain this a bit, there is no reason why allowed values for
SSH_VARIANT must be "putty" and "tortoiseplink".  An alternative
design could be "port_option=-p,needs_batch=yes" and it may be more
logical and futureproof if a variant of tortoiseplink decides to use
"-p" instead of "-P" and still require "-batch".

Prematurely attempting to share code, only because the current
vocabularies for two distinct concepts happen to overlap, is not
de-duplicating the code for maintainability.  It is adding
unnecessary work other people need to do in the future when they
want to extend the system.

Except, of course, that your hypothetical port_option and needs_batch
settings would be handled at a different point altogether.

I sense very strongly that this discussion has taken a very emotional
turn, which is detrimental to the quality. So let's take a break here.

Ciao,
Johannes