Re: [PATCH v6 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 06, 2018 at 03:04:53AM -0400, Eric Sunshine wrote:
> On Fri, Apr 6, 2018 at 2:39 AM, Taylor Blau <me@xxxxxxxxxxxx> wrote:
> > [...]
> > In this patch, we support `--type=<int|bool|bool-or-int|...>` in
> > addition to `--int`, `--bool`, and etc. This allows the aforementioned
> > upcoming patch to support querying a color value with a default via
> > `--type=color --default=...`, without squandering `--color`.
> >
> > Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> > ---
> > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> > @@ -38,12 +38,12 @@ existing values that match the regexp are updated or unset.  If
> > +The `--type` option requests 'git config' to ensure that the configured values
> > +associated with the given variable(s) are of the given type. When given
> > +`--type`, 'git config' will ensure that the variable(s) are of the given type
> > +and convert the value to the canonical form. If no type specifier is passed, no
> > +checks or transformations are performed on the value. Callers may unset any
> > +pre-existing type specifiers with `--no-type`.
>
> Sorry for being such a stickler, but this is still too mushy. The
> first two sentences are saying effectively the same thing. One or the
> other should be dropped or they should somehow be combined in a way
> that says everything that needs to be said without repetition.

I agree, and please do not apologize for being a stickler. I think that
it's important we reach consensus before landing this, since other
humans will read this.

How do you feel about?

  The `--type=<type>` option instructs 'git config' to ensure that
  incoming and outgoing values are canonicalize-able under the given
  <type>.  If no `--type=<type>` is given, no canonicalization will be
  performed. Callers may unset the existing `--type` specifier with
  `--no-type`.

I think documenting that `--no-type` unsets any pre-existing `--type`
specifier is worthwhile. That said, I also think that there's a way to
combine the last two sentences, but it might be clearer as two smaller
pieces rather than one big one.

If this is still off base, could you provide some pointers on how you
would word it?

> If necessary, iterate over updates of this paragraph here in the email
> thread until a polished version is reached rather than re-rolling the
> entire series every few minutes.

Thanks, and will do :-). I am quite new to this and was unaware of the
situations when re-rolling is and isn't desirable. I am going to wait to
re-roll this series until it has gathered more feedback, or at least
consensus, after which I think it will be ready for queueing as-is.

Thanks for your patience and guidance.


Thanks,
Taylor



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux