Re: [PATCH 1/5] Documentation/remote-helpers: Rewrite description

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 18, 2010, 02:29 UTC
Message-ID: <20100418022900.GA1669@progeny.tock>
In-Reply-To: <1271551934-sup-1118@kytes>

I’m not a remote helper expert, but I’ll review it to get the ball rolling. I assume you’ve already gone through this a lot. Most of what’s left is nitpicks.

Ramkumar Ramachandra wrote:

> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt

[...]

Show 8 quoted lines

> -These programs are normally not used directly by end users, but are
> -invoked by various git programs that interact with remote repositories
> -when the repository they would operate on will be accessed using
> -transport code not linked into the main git binary. Various particular
> -helper programs will behave as documented here.
> +Remote helper programs are normally not used directly by end users,
> +but are invoked by git when it needs to interact with remote
> +repositories.

nitpick: comma usage. Probably the simplest fix would to add a subject after ", but".

  Remote helper programs are normally not used directly by end-users,
  but they are invoked by git when it needs to interact with remote
  repositories over transports git does not support natively.

Show 5 quoted lines

> +They implement a subset of the capabilities documented
> +here, and conform to the "remote helper protocol". When git needs
> +needs to interact with a repository served by a remote helper, it
> +spawns the helper as an independent process and interacts with it over
> +the specified protocol.

Pedantic usage nitpick: I think by “interacts with it over the specified protocol” here, you mean “interacts with it according to the aforementioned protocol”. It would probably be simpler to skip mention of the protocol altogether, like this:

  A given helper will implement a subset of the commands documented
  here.  When git needs to interact with a repository using a remote
  helper, it spawns the helper as an independent process, sends
  commands to the helper’s standard input, and expects results from
  the helper’s standard output.  Because a remote helper runs as an
  independent process from git, there is no need to re-link git to add
  a new helper, nor any need to link the helper with the
  implementation of git.

[cutting everything up to]

> +
> +All the capabilities of remote helpers have to do with discovering and
> +updating remote refs, transporting objects between local and remote,
> +and updating the local object store.

Odd phrasing: this does not actually explain what capabilities are.

  Every helper must support the "capabilities" command, which git will
  use to determine what other commands the helper will accept [*].  Other
  commands generally concern facilities like discovering and updating
  remote refs, transporting objects between the object database and
  the remote repository, and updating the local object store.

[*] and to read in refspecs for remote-tracking branches. This seems silly (and would not scale well too many refspecs) --- why doesn’t git take responsibility for reading the refspecs from .git/config itself?

> +Using the 'fetch' capability,
> +they can discover refs on the remote, transfer objects from the remote
> +reachable via those refs to local, and update the local object
> +store.

Does “they” refer to the helpers or their capabilities? Best to avoid the question:

  Helpers supporting the 'fetch' capability can discover refs from the
  remote repository and transfer objects reachable from those refs to
  the local object store.

The original phrasing separated transfer from inclusion in the object db, but it is not clear to me why.

> +Using the 'push' capability, they can transfer objects from
> +local to remote, and update the corresponding refs as necessary.

It seems confusing to talk about corresponding refs here: if two branch heads point to the same commit, which is the corresponding ref?

  Helpers supporting the 'push' capability can transfer local objects
  to the remote repository and update remote refs.

Show 5 quoted lines

> +
> +Git comes with a "curl" family of remote helpers, that handle various
> +transport protocols, such as 'git-remote-http', 'git-remote-https',
> +'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
> +'fetch', 'option', and 'push'.

With the above changes (or whatever subset you think is suitable),

  Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 15214ae..9d86c26 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -13,25 +13,26 @@ DESCRIPTION
 -----------
 
 Remote helper programs are normally not used directly by end users,
-but are invoked by git when it needs to interact with remote
-repositories. They implement a subset of the capabilities documented
-here, and conform to the "remote helper protocol". When git needs
-needs to interact with a repository served by a remote helper, it
-spawns the helper as an independent process and interacts with it over
-the specified protocol. git sends commands to the helper
-over standard input, and receives the result written to standard
-output by the helper over a pipe. Because a remote helper runs as
-an independent process to interact with remote repositories, there
-is no need to re-link git when adding a new helper nor need to link
-the helper with the implementation of git.
+but they are invoked by git when it needs to interact with remote
+repositories git does not support natively.  A given helper will
+implement a subset of the capabilities documented here. When git
+needs to interact with a repository using a remote helper, it spawns
+the helper as an independent process, sends commands to the helper's
+standard input, and expects results from the helper's standard
+output. Because a remote helper runs as an independent process from
+git, there is no need to re-link git to add a new helper, nor any
+need to link the helper with the implementation of git.
 
-All the capabilities of remote helpers have to do with discovering and
-updating remote refs, transporting objects between local and remote,
-and updating the local object store. Using the 'fetch' capability,
-they can discover refs on the remote, transfer objects from the remote
-reachable via those refs to local, and update the local object
-store. Using the 'push' capability, they can transfer objects from
-local to remote, and update the corresponding refs as necessary.
+Every helper must support the "capabilities" command, which git will
+use to determine what other commands the helper will accept.  Other
+commands generally concern facilities like discovering and updating
+remote refs, transporting objects between the object database and
+the remote repository, and updating the local object store.
+
+Helpers supporting the 'fetch' capability can discover refs from the
+remote repository and transfer objects reachable from those refs to
+the local object store. Helpers supporting the 'push' capability can
+transfer local objects to the remote repository and update remote refs.
 
 Git comes with a "curl" family of remote helpers, that handle various
 transport protocols, such as 'git-remote-http', 'git-remote-https',