Re: [PATCH v2 10/16] tag: change misleading --list <pattern> documentation

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 22, 2017, 22:08 UTC
Message-ID: <CACBZZX4rofOquS4q0NDCb8cHOF=E7iPzFMxj3bre6CTB7=b_MA@mail.gmail.com>
In-Reply-To: <xmqq1stpj9xv.fsf@gitster.mtv.corp.google.com>

On Wed, Mar 22, 2017 at 10:09 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Please drop "interleaved", as we are not encouraging GNUism.  We
>>> just tolerate it but do not guarantee to keep them working.
>>
>> This brings up the same point you made in
>> <xmqqmvci2zoc.fsf@gitster.mtv.corp.google.com>, I replied to in
>> <CACBZZX5LN8nhs85K18XVg4Y9_qb9zKGBoFnnQHSsRZVOP=OkDw@mail.gmail.com>,
>> and that you didn't get back to me about.
>>
>> Rather than split the discussion or me copy/pasting large parts of
>> that E-Mail could you please reply to me over in that thread?
>
> There is nothing to reply.  We know some people favor GNUism, but we
> do not actively encourage it, period.

I'd understand the objection to encouraging if I was proposing to include mentions of "--list <pattern> --list <pattern>" in the documentation, but what this patch is about (and this wasn't clear enough from the beginning, I'll reword it), is essentially:

"So for the last 5 years of Git releases if you'd read the git-tag docs you'd think `--list <pat> --list <pat>` was the blessed way of supplying multiple patterns. That's not actually needed, or encouraged, so here's a change to the docs & a test addition to make sure we know if we break this long-documented pattern in the future".

You'd like the doc change but not the regression test. If so I'm fine with that. The reason I'm asking you follow-up questions about it is because I'd like to know in general, for future submissions, what sort of things you think we should be putting in the test suite. I.e. should the tests be:

a) Only be a collection of invocations of git we'd be comfortable showing to someone as "this works, and this is how you should do it", or things that explicitly fail marked with test_must_fail.

b) or a) && also various surprising combinations of things we don't necessarily want to encourage or even support in the future, but which are in there so if we change them, we at least know our change changed something that worked before.

Now. I think the policy should be "b". The main reason is that as I noted in [1] people do write scripts against the porcelain. There's doubtless scripted invocations out there that use multiple GNU-like "-l <pattern>" options, and it's worthwhile for us to know if we make a change that breaks that.

But if you think it should be "a" obviously that's the project policy, but then I'd like to know that it's "a", both because it'll save me time authoring future patches, and I'd like to submit some change to SubmittingPatches or t/README describing to others what sort of tests are acceptable for inclusion.

The reason I brought up a hypothetical "test_might_get_deprecated" feature in my original message [2] is that even if you think we should only do "a" now and not "b", I think that perhaps that's only because of some limitation of the current test suite. I.e.:

1) Maybe we shouldn't run these ("b" && !"a") tests by default unless
some flag is provided, so that someone who's working on an otherwise
worthwhile new feature isn't dissuaded by some failing test that's
checking for an obscure and historically unintended feature of git.

2) Or we could run them, but e.g. turn them into TODO's for failure to
mark them as things we'd like to deprecate.

1. <CACBZZX59KXPOEjiUKtZLN6zjO_xpiWve7Xga6q-53J2LwvfZyw@mail.gmail.com>
2. <CACBZZX5LN8nhs85K18XVg4Y9_qb9zKGBoFnnQHSsRZVOP=OkDw@mail.gmail.com>,