Re: [PATCH 2/7] builtin/bugreport.c: create '--diagnose' option
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-08-02 02:26:51
On Mon, Aug 01 2022, Victoria Dye via GitGitGadget wrote:
From: Victoria Dye <redacted> [...] Documentation/git-bugreport.txt | 11 +- builtin/bugreport.c | 282 +++++++++++++++++++++++++++++++- t/t0091-bugreport.sh | 20 +++ 3 files changed, 309 insertions(+), 4 deletions(-) [...]
Maybe it's not easy in this case, but I wonder if this series can't be re-arranged in a way that more directly benefits from the diff move detection. E.g. if we moved the unchanged functions to a new repo-disk-usage.c or something we could have an intermediate step of having both use that, and then going forward would work towards a better lib/built-in split-up...
quoted hunk ↗ jump to hunk
--- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt@@ -8,7 +8,7 @@ git-bugreport - Collect information for user to file a bug report SYNOPSIS -------- [verse] -'git bugreport' [(-o | --output-directory) <path>] [(-s | --suffix) <format>] +'git bugreport' [<options>][...] static const char * const bugreport_usage[] = { - N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"), + N_("git bugreport [<options>]"), NULL };
We have some built-ins that punt on re-listing the synopsis in the -h output, but we always list the full usage in the SYNOPSIS. I think both of these hunks should be dropped, instead we should (presumably) add a "git bugreport --diagnose" to this, and if it combines (or not) with other options, let's update both accordingly.
[...] + strvec_pushl(&archiver_args, "scalar-diagnose", "--format=zip", NULL);
Is the "scalar-diagnose" here a mistake?