Re: [PATCH v3 09/11] builtin/bugreport.c: create '--diagnose' option
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Aug 11, 2022, 20:30 UTC
- Message-ID
- <220811.86fsi2plk7.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <f9a5f658-7c6f-bb3a-d6df-0ab7d2908395@github.com>
On Thu, Aug 11 2022, Victoria Dye wrote:
Show 23 quoted lines
> Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Aug 10 2022, Victoria Dye via GitGitGadget wrote:
>>
>>> From: Victoria Dye <vdye@github.com>
>>>
>>> Create a '--diagnose' option for 'git bugreport' to collect additional
>>> information about the repository and write it to a zipped archive.
>>> [...]
>>> 'git bugreport' [(-o | --output-directory) <path>] [(-s | --suffix) <format>]
>>> + [--diagnose[=<mode>]]
>>> [...]
>>> static const char * const bugreport_usage[] = {
>>> - N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
>>> + N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>] [--diagnose[=<mode>]"),
>>> NULL
>>> };
>>
>> This still has the SYNOPSIS v.s. -h discrepancy noted in
>> https://lore.kernel.org/git/220804.86v8r8ec4s.gmgdl@evledraar.gmail.com/
>
> The discrepancy you pointed out was on 'git diagnose' (which has since been
> fixed),Ah, sorry. I missed that & conflated the two.
Show 6 quoted lines
> this is a pre-existing one in 'git bugreport'. I decided against > fixing *this* one because it didn't really fit into any of the patches in > this series, so it would need its own patch. When balancing "leave things > better than you found them" vs. "stay focused on the purpose of the series", > I leaned towards the latter to avoid setting a precedent for other 'git > bugreport'-related scope creep.
In any case, I'm pointing out the difference in one of them having \n-wrapping inconsistent with the other, which is an addition in this series, sorry about not being clear.
I see that there's also the difference in how they format "--suffix", but that's pre-existing & we can leave it for now. I think that's what you're pointing out here as pre-existing.
> If you have the patches to audit this sort of thing, I think a nice place to > fix this might be in a dedicated series fixing discrepancies tree-wide. Even > better, you could include the patches in your tree that detect them as part > of e.g. the 'static-analysis' CI job.
Yeah I do have those, and will probably submit those sooner than later, and I'll end up spotting differences once they land on "master" (e.g. [1] is one such case).
But this one is just one I eyeballed during review.
1. https://lore.kernel.org/git/220811.86o7wrov26.gmgdl@evledraar.gmail.com/