Re: [PATCH 2/2] docs: handling-regressions.rst: clarify that "Closes:" tags work too

[Thorsten Leemhuis]

On 02.04.24 12:13, Karel Balej wrote:
> thank you very much for your feedback.

yw!

> Thorsten Leemhuis, 2024-04-02T11:27:57+02:00:
>> On 01.04.24 17:19, Randy Dunlap wrote:
>>> On 4/1/24 1:38 AM, Thorsten Leemhuis wrote:
>>>> On 28.03.24 20:29, Karel Balej wrote:
>>>>> The regressions handling manual claims that regzbot associates patches
>>>>> fixing an issue with the report based on the occurrence of the
>>>>> appropriate "Link:" trailers. It reasons that this does not add any
>>>>> burden on the maintainers/bug fix authors as this is already mandated by
>>>>> the "Submitting patches" guide. In fact however, the guide encourages
>>>>> using "Link:" tags for related discussions or issues which the patch
>>>>> fixes only partially, recommending "Closes:" for full resolutions.
>>>>>
>>>>> Despite it not being mentioned anywhere in the "Handling regressions"
>>>>> guide, regzbot does in fact take the "Closes:" tags into account and
>>>>> seems to in fact treat them fully equivalently to "Link:" tags.
>>>>>
>>>>> Clarify this in the regressions handling guide by always mentioning both
>>>>> of the tags.
>>>>
>>>> Many thx for this and the other patch. I had planned to do something
>>>> like this myself, but never got around to.
>>>>
>>>> There is just one thing that makes me slightly unhappy: this tells
>>>> readers that they can use both, but leaves the question "what's the
>>>> difference" respectively "in which situation should I use one or the
>>>> other" unanswered.
> 
> I see your point and I agree. I have perceived something similar when
> editing the document: I wondered whether it's really good to *always*
> spell out both variants or whether it would perhaps be enough in some
> places only.
> 
> I think the way that I ultimately did it counts on the reader being
> familiar with the "Submitting patches" document and knowing the "true"
> meanings of both Closes: and Link: and when to use each. So my goal was
> only to mention it because the way it was written seemed to almost imply
> to me that Closes: does *not* work and is thus not recommended which
> seemed in conflict with the "Submitting patch" guide, which was even
> more confusing since it literally referred to it.
> 
> In other words, it wasn't actually my goal to answer that question you
> pose, because that is already answered in the other document.

Yeah, but documents ideally should work on their own, so offering two
solutions without at least hinting at the differences is not ideal.

> I also didn't want to be too drastic with the changes because the
> prevalence of Link: seemed so strong that I thought that I must be
> missing something and that you have a good reason to write it like this.

Just history: when that document was written we didn't have a "Closes"
tag yet.

> So I wanted to stay safe :-)

:-)

> Anyway, if you are OK with that, I can definitely change it to Closes:
> everywhere and only mention Link: marginally, saying that it works too
> and explaining the difference while referring the reader to "Submitting
> patches" for more information (not that there would be too much more on
> this subject).

Yeah. Maybe just write something along the lines of "using a 'Link:' tag
instead  works for now as well, as some subsystems prefer it over the
younger 'Closes:' tag" somewhere -- while using Closes: everywhere else.

> Of course I wouldn't want to for example delay you so if you get around
> to it sooner than I will then feel free to make the changes either as a
> modification of this patch or just on its own.

I doubt that will happen.

> Perhaps you could take the first patch already if you have no
> reservations there and I will then just send v2 of this one?

I'll send a reviewed-by for that, but I guess Jonathan will take that
for the next merge window anyway. So there is still some time to prepare
a updated series with both changes. And if time is running out, we can
still ask Jonathan to pick up the first patch.

>>>> I also find the patch description a bit verbose; and it would be
>>>> good to turn the text upside down: first outline what the patch,
>>>> then maybe describe the "why".
> 
> I actually probably like it more this way. After all, the outline (or
> "what") is the patch subject, everything that comes after it in the body
> is usually meant to explain "why". But sure, I can swap it if you want
> :-)

Well, imagine you have to read 25 shorts text that each explain the
"why" first and then the "what" while you only care about one certain
change. Then your preferred approach quickly becomes annoying, because
you have to read a lot of "why" for every patch before you can decide if
you actually care about it.

> As for the verbosity, I will keep it in mind when working on v2,
> although I also generally don't consider verbosity a bad thing. I might
> have been too verbose, though, because as I have mentioned, I was
> confused why it isn't like this already and wanted to offer my full
> reasoning so that I could be shown where I err :-)

:-)

> One more thing that I wanted but ultimately forgot to mention in the
> cover letter: thank you very much for writing these guides in the first
> place, I find them very instructive and useful.

Thx for saying that, I often wonder if they are really worth the trouble...

Ciao, Thosten