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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 7, 2018, 08:25 UTC
Message-ID: <CAPig+cQXJuLWpxfwhQ98a23wfAAzKZpkOvqWBdXaKKHLiW-7Uw@mail.gmail.com>
In-Reply-To: <20180407003912.GA78042@syl.local>

On Fri, Apr 6, 2018 at 8:39 PM, Taylor Blau <me@ttaylorr.com> wrote:

Show 13 quoted lines

> On Fri, Apr 06, 2018 at 03:04:53AM -0400, Eric Sunshine wrote:
>> 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.
>
> 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`.

This sounds fine. (Maybe s/the existing/an existing/)

> 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.

Smaller, simpler, more easily digested sentences are a win.

Show 8 quoted lines

>> 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,

Just as it's a burden for you to repeatedly re-roll, it's also a burden on reviewers to repeatedly re-read the series. Ideally, we'd like fewer, rather than more, re-rolls, so it's good to nail down questionable or ambiguous issues via normal email discussion before going for a re-roll, and some reviewers try to assist with deciding if a re-roll is needed by saying whether or not a review comment warrants one.

Allowing time for others to review and possibly comment on a series before re-rolling is indeed a good idea.

Another useful tactic is to include, in the cover letter, an interdiff between the previous and new versions, which gives reviewers a way to quickly examine the changed parts of the series without necessarily having to re-review each patch in entirety (an often time-consuming task).

> after which I think it will be ready for queueing as-is.

Famous last words ;-)