git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tim Henigan <tim.henigan@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: th/remote-usage
Date: Wed, 18 Nov 2009 16:28:33 -0500	[thread overview]
Message-ID: <32c343770911181328v6d61967bna165f08b9e58d5be@mail.gmail.com> (raw)
In-Reply-To: <20091118114808.GA13346@progeny.tock>

Hi and thanks for your review.

On Wed, Nov 18, 2009 at 6:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>> [New Topics]
>>
>> * th/remote-usage (2009-11-16) 1 commit.
>>  - git remote: Separate usage strings for subcommands
>
> Glancing at pu^2, I had two small nitpicks: [<options>...] is five
> characters longer than strictly necessary

I based my patch on what I found in other builtin functions (such
as push and diff).  That being said, I don't think that either my
original patch or your updated version is completely correct.
The choices seem to be:
  (1) [<options>...]:  My original based on my interpretation of
      IEEE 1003.1. [1]
  (2) [options]: Your proposal, which drops both the '<>' and '...'.
  (3) <options>:  Used in builtin-diff.c.  Which does not show
      that the options are -- optional.
  (4) [<options>]: What I now believe is correct (based on the
      current implementation of builtin-push.c).  This drops the
      '...' which IEEE 1003.1 defines as allowing multiple options
      to be specified, but it conforms to the conventions in other
      commands.

There does not (yet) seem to be consistency in how options
are presented.  My current plan is to change the patch to
use choice #4, but if Junio has a chance to comment, I will
of course defer to his decision.

I will send an updated patch that implements choice #4 as
soon as I can (should be within the next 12 hours).


> and the argument to git
> remote set-head is not actually optional.

This was obviously an oversight on my part.  I will include the
fix in the next version.


...and from your second email:
> Another option would be to make the strings into static
> variables.

Thanks for the analysis, but I don't plan to include this change
unless specifically requested.


[1]: http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html

  parent reply	other threads:[~2009-11-18 21:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-18  7:53 What's cooking in git.git (Nov 2009, #04; Tue, 17) Junio C Hamano
2009-11-18  8:22 ` Tay Ray Chuan
2009-11-18  8:40 ` Johannes Sixt
2009-11-19 18:11   ` Junio C Hamano
2009-11-18 11:48 ` th/remote-usage Jonathan Nieder
2009-11-18 12:05   ` th/remote-usage Jonathan Nieder
2009-11-18 21:28   ` Tim Henigan [this message]
2009-11-18 14:43 ` What's cooking in git.git (Nov 2009, #04; Tue, 17) Nguyen Thai Ngoc Duy
2009-11-18 16:12   ` Johannes Sixt

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32c343770911181328v6d61967bna165f08b9e58d5be@mail.gmail.com \
    --to=tim.henigan@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /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.
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 NNTP newsgroup(s).