On Thu, Aug 04 2022, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> Create a 'git diagnose' builtin to generate a standalone zip archive of
> repository diagnostics.
It's good to have this as a built-in separate from "git bugreport",
but...
> +git-diagnose - Generate a zip archive of diagnostic information
...I'd really prefer for this not to squat on such a common name we
might regret having reserved later for such very specific
functionality. I'd think e.g. these would be better:
git mk-diagnostics-zip
Or maybe:
git archive-interesting-for-report
If I had to guess what a "git diagnose" did, I'd probably think:
* It analyzes your config, and suggests redundancies/alternatives
* It does some perf tests / heuritics, and e.g. suggests you turn on
the commit-graph writing.
etc., this (arguably even too generic then) made sense as "scalar
diagnose" because scalar is all about being an opinionated interface
targeted at performance", so there's an implied "my repo's performance"
following a "scalar diagnose".
But as a top-level command-name I think we should pick something more
specific to what it does, which is (I haven't fully read ahead in the
re-roll, but I'm assuming is) mostly/entirely to be a "helper" for
"scalar diagnose" and/or "git bugreport".
> + switch (safe_create_leading_directories(zip_path.buf)) {
> + case SCLD_OK:
> + case SCLD_EXISTS:
> + break;
> + default:
> + die(_("could not create leading directories for '%s'"),
> + zip_path.buf);
This seems to be carrying forward a minor bug from bugreport.c we should
probably fix: we should use die_errno() here (and maybe lead with a
commit to fix bugreport.c's version).
The strbuf*() before that also seems like a good candidate for a utility
function in your new diagnose library, i.e. to have both bugreport.c and
diagnose.c pass it the prefix/suffix/format, then try to create that
directory, then replace the copy/pasting here with a one-line call to
that now-shared function.
The two codepaths only seem to differ in the prefix & suffix from a
quick skim, the rest is all copy/pasted...
