Subject: Re: [RFC PATCH v3 10/17] Move WebDAV HTTP push under remote-curl

From: "Shawn O. Pearce" <[email-masked]>
Date: Tue, 27 Oct 2009 18:08:31 -0700

Tay Ray Chuan <[email-masked]> wrote:
> On Thu, Oct 15, 2009 at 11:36 AM, Shawn O. Pearce <[email-masked]> wrote:
> > diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> > @@ -34,6 +34,10 @@ Commands are given by the caller on the helper's standard input, one per line.
> >        value of the ref. A space-separated list of attributes follows
> >        the name; unrecognized attributes are ignored. After the
> >        complete list, outputs a blank line.
> > ++
> > +If 'push' is supported this may be called as 'list for-push'
> > +to obtain the current refs prior to sending one or more 'push'
> > +commands to the helper.
> 
> The new paragraph should have the same indentation as 'list'.

Isn't the style I have here the standard way of adding a second
paragraph to a list item?
 
> It would have been great if you implemented this as a filter, such that
> 'list <attr>' lists the refs with the specified attribute <attr>,
> rather than hardcoding it.

Ack, will fix.
 
> > @@ -59,6 +63,22 @@ suitably updated.
> > +When the push is complete, outputs one or more 'ok <dst>' or
> > +'error <dst> <why>?' lines to indicate success or failure of
> > +each pushed ref.  The status report output is terminated by
> > +a blank line.  The option field <why> may be quoted in a C
> > +style string if it contains an LF.
> 
> You should mention that this behaviour only occurs when the
> --helper-status option is used.

No.  This manual page is about what a helper program must do, so I
mean what "git-remote-curl" must do.  The fact that git-remote-curl
honors this output format via passing the --helper-status option
into a command it executes is an implementation detail.

> > @@ -106,6 +132,11 @@ OPTIONS
> > +'option dry-run' \{'true'|'false'\}:
> > +       If true, pretend like the operation completed successfully,
> > +       but don't actually change any repository data.  For most
> > +       helpers this only applies to the 'push', if supported.
> > +
> 
> The 'like' after 'pretend' can be, like, removed. :)

Thanks, fixed.
 
> Two more areas in http-push.c that should have status messages
> (generated on top of pu):

I folded your patches into my series, thanks.
 
-- 
Shawn.