Re: [PATCH 2/2] Add a remote helper to interact with mediawiki (fetch & push)

Hi Matthieu,

Matthieu Moy wrote:

Jonathan Nieder [off-list ref] writes:

quoted

Here's an old attempt to make the documentation a little easier to read,
and hopefully also to add to.

Thanks, that helps. I wish we had all this earlier ;-).

Glad you like it.

quoted

+Git sends the remote helper a list of commands on standard input, one
+per line.  The first command is always the 'capabilities' command, 

Do we want to set this in stone?

Yes, I think we should set it in stone.  Helpers can use that as a
sanity check to know they are actually being used as a remote
helper.  And specifying it makes the protocol more concrete.

Wouldn't a Git implementation calling
"option" before "capabilities" be correct?

At that point git doesn't know if the helper implements the "option"
capability.

quoted

+Capabilities
+~~~~~~~~~~~~

(perhaps name the section "Overview of Capabilities"?)

Yes, good idea.

[...]

Since this "Capabilities" section is meant to be an overview, I'd
shorten this to

+'refspec' <refspec>::
+	This modifies the 'import' capability, allowing the produced
+	fast-import stream to modify refs in a private namespace
+	instead of writing to refs/heads or refs/remotes directly.

and drop the detailed explanation here.

Another good idea.

quoted

+Capabilities for Fetching
+~~~~~~~~~~~~~~~~~~~~~~~~~

[...]

quoted

+'refspec' <refspec>::
+	This modifies the 'import' capability.

Since this would be the "detailed explanation" part, this is the one
readers will read more carefully, so I'd put the recommandation right
here:

+	It is recommended that all importers providing the 'import'
+	capability use this.

Yet another. :)

and of course, keep this:

quoted

++
+A helper advertising
+`refspec refs/heads/{asterisk}:refs/svn/origin/branches/{asterisk}`
+in its capabilities is saying that, when it handles
+`import refs/heads/topic`, the stream it outputs will update the
+`refs/svn/origin/branches/topic` ref.
++
+This capability can be advertised multiple times.  The first
+applicable refspec takes precedence.  The left-hand of refspecs
+advertised with this capability must cover all refs reported by
+the list command.  If no 'refspec' capability is advertised,
+there is an implied `refspec {asterisk}:{asterisk}`.
+

Thanks much!  Will try to find time to resend later today.