From: "Torsten Bögershausen" <tboegi@web.de>
To: Mike Hommey <mh@glandium.org>, git@vger.kernel.org
Cc: gitster@pobox.com
Subject: Re: [RFC PATCH 2/3] connect: group CONNECT_DIAG_URL handling code
Date: Fri, 29 Apr 2016 22:00:52 +0200 [thread overview]
Message-ID: <632a7629-645b-43a7-abd7-fcbc35362f60@web.de> (raw)
In-Reply-To: <1461890625-23222-2-git-send-email-mh@glandium.org>
On 29.04.16 02:43, Mike Hommey wrote:
> get_host_and_port(&ssh_host, &port);
> + if (!port)
> + port = get_port(ssh_host);
I'm not sure, if this is an improvement or not.
The original intention was, to check what the parser did, before
going out to the external program (like ssh) or opening a socket.
Everything should be prepared, as if the action had been taken,
and in the last nanosecond we jump out and print what would have
been done.
(The nanosecond may depend on the CPU speed)
In any case, for SSH there should be only one line calling get_host_and_port(),
or get_port().
This is to check the complete logic path in test cases, as well as it allows
the user to check the URL, especially when using SSH.
Splitting the logic into a path where we just print and one, where
the connection is really made, looses "single point of truth" - we don't know
any longer, what we test or print for diag.
next prev parent reply other threads:[~2016-04-29 20:01 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-28 14:12 [RFC PATCH 0/4] git_connect: add some flexibility Mike Hommey
2016-04-28 14:12 ` [PATCH 1/4] git_connect: extend to take a pseudo format string for the program to run Mike Hommey
2016-04-28 14:12 ` [PATCH 2/4] git_connect: avoid quoting the path on the command line when it's not necessary Mike Hommey
2016-04-28 16:14 ` Stefan Beller
2016-04-28 14:12 ` [PATCH 3/4] git_connect: allow a file descriptor to be allocated for stderr Mike Hommey
2016-04-28 14:12 ` [PATCH 4/4] git_connect: add a flag to consider the path part of ssh urls relative Mike Hommey
2016-04-28 17:41 ` [RFC PATCH 0/4] git_connect: add some flexibility Junio C Hamano
2016-04-28 23:29 ` Mike Hommey
2016-04-29 0:43 ` [RFC PATCH 1/3] connect: make parse_connect_url public Mike Hommey
2016-04-29 0:43 ` [RFC PATCH 2/3] connect: group CONNECT_DIAG_URL handling code Mike Hommey
2016-04-29 15:59 ` Junio C Hamano
2016-04-29 17:13 ` Junio C Hamano
2016-04-29 20:00 ` Torsten Bögershausen [this message]
2016-04-29 0:43 ` [RFC PATCH 3/3] connect: move ssh command line preparation to a separate (public) function Mike Hommey
2016-04-29 16:58 ` Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=632a7629-645b-43a7-abd7-fcbc35362f60@web.de \
--to=tboegi@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mh@glandium.org \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).