Re: [PATCH] diagnose: require repository

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 19, 2023, 18:09 UTC
Message-ID: <xmqqo7guwkla.fsf@gitster.g>
In-Reply-To: <CAN0heSqmZ7QXJbet2Tp=YYCjBLToOHtNy+n=zcf29XYaukYN0w@mail.gmail.com>

Martin Ågren <martin.agren@gmail.com> writes:

> Correcting myself: The zip archive would actually contain
> `diagnostics.log` with some general info about the machine and Git
> build.

So it could contain some useful information without a specific repository, perhaps.

> Good point. TBH, I had no idea about `git bugreport --diagnose`.

You are not alone ;-) I didn't, either. Before responding to your patch, that is.

Show 16 quoted lines

>> +       if (!startup_info->have_repository && diagnose != DIAGNOSE_NONE) {
>> +               warning(_("no repository--diagnostic output disabled"));
>> +               diagnose = DIAGNOSE_NONE;
>> +       }
>> +
>
> When the user explicitly provides that option, it seems unfortunate to
> me to drop it. Yes, we'd warn, but `git bugreport` then pops a text
> editor, so you would only see the warning after finishing up the report.
> (Maybe. By the time you quit your editor, you might not consider
> checking the terminal for warnings and such.)
>
> So I'm inclined to instead just die if we see the option outside a repo.
> If `diagnose` the command fundamentally requires a repo (as with my
> patch) it seems surprising to me to not have `--diagnose` the option
> behave the same.

I have no strong opinion. Victoria is on Cc: already, whose name appears a lot more often than mine in the shortlog for "diagnose" stuff, so I'll defer to her area expertise.

Thanks.