Ævar Arnfjörð Bjarmason wrote:

On Mon, Aug 01 2022, Victoria Dye via GitGitGadget wrote:

quoted

[...] I didn't see a major UX benefit of 'git diagnose' vs 'git
bugreport --diagnose', so I went with the latter, simpler approach.

I really wanted to like this, but I find the end result here really
confusing from a UX perspective.

You can now run "git bugreport --diagnose", which creates a giant *.zip
file to go along with your *.txt, but your *.txt makes no reference to
it.

Should you ... attach it to your bug report to this mailing list, do
something else?

The documentation doesn't offer much in the way of hints, other than
suggesting (with --no-report) that this --diagnose is for something
entirely different (and that's how "scalar" uses it).

I know what it's really for after reading this series, but for "git
bugreport" in particular we should be really careful about not making
the UX confusing.

The generated *.zip contains some really deep info about your repo (and
not just metadata, e.g. copies of the index, various logs etc.), someone
e.g. in a proprietary setting really doesn't want to be sharing that
info.

So I would like to see real integration into "git bugreport", i.e. for
us to smartly report more repository metrics, e.g. approx number of
loose objects, the sort of state "__git_ps1" might report, etc.

But I think the end-state here makes things much more confusing for
users.

The "confusing UX" you describe here doesn't seem to be an inherent issue
with the implementation (nor is it as insurmountable as you're implying), it
largely appears to be an issue of under-documentation. I'll improve that in
V2 [1], but I want clarify what I was/am going for here as well.

In the context of a bug report, the diagnostics are intended to be used as
supplemental information to aid in debugging (i.e., attached with the report
in the email to the list). They're especially valuable when a bug reporter
isn't very familiar with Git internals and they can't reproduce the issue. A
lot of bugs can be investigated without those diagnostics, though, which is
why '--diagnose' isn't "on" by default.

There are also valid use-cases (beyond 'scalar diagnose') for '--no-report':
someone requests more information after looking into an already-generated
report, or a developer wants to investigate a bug on their own and use the
diagnostics as a "starting point" to guide more in-depth debugging. 

As for the proprietary data issue, I'd be open to having an option to
configure which diagnostics a user wants (either something like '--diagnose
<level>' or a separate option entirely). I'm pretty indifferent on the UI,
though, so I'll defer to other contributors on 1) if they want that feature,
and 2) what they think that should look like.

[1] https://lore.kernel.org/git/f3235afe-25cc-21a4-fc35-56e35d6be0ce@github.com/ (local)

quoted

An alternative implementation considered was creating a new 'git diagnose'
builtin, but the new command would end up duplicating much of
'builtin/bugreport.c'.

It seems we always "return" from cmd_bugreport() quite quickly, and we
basically only share the code to create the output directory. Just
duplicating or sharing that seems like a much better approach for now
than creating the above UX confusion.

Note that you can also share code between multiple built-ins, even in
the same file (see e.g. builtin/{checkout,log}.c). So we could even
share something like the safe_create_leading_directories() calling code
in bugreport.c without libifying it.

You deleted the part where I addressed this suggestion directly:

Although that issue could be overcome with refactoring, I didn't see a
major UX benefit of 'git diagnose' vs 'git bugreport --diagnose', so I
went with the latter, simpler approach.

And, in the process of writing down my thoughts on the UX above, I've become
more convinced that including '--diagnose' in 'git bugreport' is the better
way to present this functionality to users.

quoted

Finally, despite 'scalar diagnose' now being nothing more than a wrapper for
'git bugreport --diagnose', it is not being deprecated in this series.
Although deprecation -> removal could be a future cleanup effort, 'scalar
diagnose' is kept around for now as an alias for users already accustomed to
using it in 'scalar'.

We don't have a "make install" to get a "scalar" onto user's systems
yet, do we really need to worry about those users?

Or is this a reference to the out-of-tree version of "scalar", not
git.git's?

In practice, it's the "out-of-tree Scalar" users that would care the most.
That said, with Scalar in the Git tree (albeit 'contrib/' and not built by
default), I think it's reasonable to want to avoid breaking changes if
possible. The continued existence of 'scalar diagnose' wouldn't really be
hurting anyone anyway, so there's no pressing need to deprecate it now.