Re: [RFC PATCH 3/3] connect: move ssh command line preparation to a separate (public) function

[Junio C Hamano <gitster@pobox.com>]

Mike Hommey <mh@glandium.org> writes:

> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---

I think moving it out into a separate helper function is a very good
change to make the end result easier to follow.

Turning the helper to extern and adding it to connect.h does not
benefit us (and [PATCH 1/3] doesn't, either).

And in the longer term, I doubt think it (and [PATCH 1/3]) would
benefit you, either.  We do not mean to make what goes into libgit.a
(which connect.o is a part of) be a stable "library interface", so
all sorts of future changes we make would affect your external code,
e.g.

 - some people periodically check for extern symbols that do not
   have to be extern, and this will be marked static again any time
   (or you somehow need to arrange it so that this function is
   marked as special to reduce false positives);

 - we may need to update the external interface to this function,
   changing parameters, or updating its return value.

even if we promise not to make such a change only for the purpose of
deliberately breaking you.  So you have to be prepared and willing
to follow the evolution of in-core code anyway.

So I would suggest restructuring this series to do

 * 2/3 (DIAG consolidation)
 * refactoring in 3/3 but not s/static/extern/

and in optional follow(s)-up, do

 * s/static/extern/ and update to *.h in 3/3

 * 1/3, but I do not think it is necessary for users of
   prepare_ssh_command()

The last patch could be kept in your fork of git.git if you want to
keep linking with libgit.a, but my feeling is that we'd prefer to do
without it at least for now, until we gain in-tree users that live
outside connect.c that want to call the helper.


Thanks.