Thread (70 messages) 70 messages, 4 authors, 2022-08-18

Re: [PATCH v3 09/11] builtin/bugreport.c: create '--diagnose' option

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-08-11 20:33:54

On Thu, Aug 11 2022, Victoria Dye wrote:
Ævar Arnfjörð Bjarmason wrote:
quoted
On Wed, Aug 10 2022, Victoria Dye via GitGitGadget wrote:
quoted
From: Victoria Dye <redacted>

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/ (local)
The discrepancy you pointed out was on 'git diagnose' (which has since been
fixed),
Ah, sorry. I missed that & conflated the two.
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/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help