guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [workflow] Automatically close bug report when a patch is committed


From: Giovanni Biscuolo
Subject: Re: [workflow] Automatically close bug report when a patch is committed
Date: Thu, 14 Sep 2023 11:42:43 +0200

Hi Maxim and Vagrant,

I'm sorry for some of the inconprehensions.

Finally I think we got a useful design overall for the _two_ user cases
and the related pieces of metadata and algorithms, now it's time for
_two_ implementations proposals, that also means _code_...

If nihil obstat, I'm going to open 2 bug reports (severity: wishlist)
and will take ownership... but only to coordinate the work, since I miss
some competence /and/ some authorization (for example to get and/or
install the server side git hook)

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

[...]

>> Anyway, I agree with Liliana that having more keyworks will help humans
>> better capture (and remember) the implied semantics (that we should
>> definitely document, anyway); for this reason my proposal is to accept
>> all this _lowercased_ keywords (all followed by a ":" with NO spaces in
>> between): fix, fixes, close, closes, resolve, resolves, do, done.
>
> OK, I now get the point; we're not discussing synonyms but various
> actions

Yes :-D

[...]

>>> If we choose this simple scheme where the top commit of a series can be
>>> annotated with Debbugs control commands, I'd opt for:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> Applies: #bug-number
>>> --8<---------------cut here---------------end--------------->8---
>>
>> Uh I think I get it: you mean we could use a keyword in the commit
>> message to allow the committer to effectively link a commit to a
>> #bug-number, right?
>
> Yes!

OK! :-)  Let's see how this relates to the 2 use cases we are talking
about:

1. Use "Fixes:" (et al) in commit msg to tell "the hook" to close the
bug.

This "action" implies that the commit we are pushing upstream "Applies:"
to that bug report; it has no added value.

2. Use 'Change-Id'...

This also implies that the commit we are pushing upstream "Applies:" to
that bug report related to that [PATCH]; no added value also.

So, when and only when we will implement a 'Change-Id' requirement
adding an 'Applies' metadata is not useful for linking [PATCH]es to a
bug report.

Did I miss something?

> I think my thought train went that way while Liliana and yours
> were more focused on a post push action to close *fixed* issues,
> right?

Yes, it's a (super?) /brainstorming/ :-)

> What I described here was a general process by which we could close
> *patches* series that were forgotten in an 'open' state.

Yes: until we miss the 'Change-Id' metadata, we cannot do [1] nothing
for forgotten patches series.

[...]

>> Namespace has been assumed as part of the proposed URI to try address
>> Vagrant's concerns [1]:
>>
>>
>>    Sooooo... I maintain the guix package in Debian, and want to make sure
>>    that whatever bug-closing indicator guix upstream uses, does not end up
>>    triggering when I push new upstream versions to salsa.debian.org ... and
>>    start incorrectly marking incorrect bug numbers on bugs.debian.org that
>>    were meant for debbugs.gnu.org.
>
> I don't understand how this risk could be triggered; we're strictly
> talking about commits made in the Guix repository, not in one of
> Debian's?  Why/how would a Guix commit message trigger a Debian Debbugs
> server action?  Maybe if Vagrant put something like:
>
> Fixes: <debbugs.debian.org/NNNNN> that could cause problems?  But then
> the URL is different, so we could filter out these, so I don't see the
> problem, if we use URLs.

Yes we are saying the same thing! :-)

Sorry I've made confusion but Vagrant's concern was expressed _before_
someone proposed (maybe Liliana) to use namespaced URIs.

Vagrant please: do you confirm that using URLs "Fixes:
<issues.guix.gnu.org/NNNNNN>" is OK for your usecase?

[...]

>> Fixes: [optional bug description] <namespace:#bug-number>
>>
>> (here, URI is <namespace>:#<bug-number>)
>>
>> ...but then you, Maxim, suggested [3] this form:
>>
>> Fixes: bug#65738 (java-ts-mode tests)
>
> Note that we can stick with the <issues.guix.gnu.org/NNNNNN> URL and
> achieve the same result

Thinking twice about this point, now I see that using the URL is
**much** better than <guix:NNNNNN>, simply because URLs can be
copy/pasted in a browser for users not using the bug-reference Emacs
feature or any other similar feature in their preferred IDE, if
available.

> with some extra config (see: bug#65883).

Fantastic!

[...]

>> The automatic email message will be sent to our "bug control and
>> manipulation server" [5], with this header:
>>
>>
>> From: GNU Guix git hook <noreply@gnu.org>
>> Reply-To: <Committer> <<committer-email>>
>> To: control@debbugs.gnu.org
>>
>>
>> and this body:
>>
>>
>> package guix
>> close <bug-number> [<commit-hash>]
>> quit
>>
>>
>> The "Reply-To:" (I still have to test it) will receive a notification
>> from the control server with the results of the commands, including
>> errors if any.
>>
>> Then, the documentation for the close command [5] states:
>
> Note that 'close' is deprecated in favor of 'done', which does send a
> reply.

Sorry I'm not finding 'done' (and the deprecation note) here:
https://debbugs.gnu.org/server-control.html

Maybe do you mean that we should not use the control server but send a
message to <bug-number>-done@debbugs.gnu.org?

Like:

--8<---------------cut here---------------start------------->8---

From: guix-commits
To: <bug-number>-done@debbugs.gnu.org

Version: <commit-hash>

This is an automated email from the git hooks/post-receive script.

This bug report has been closed on behalf of <Committer>
<<committer-email>> since he added an appropriate pseudo-footer in the
commit message of <commit-hash> (see <http link to documentation>).

For details on the commit content, please see: <http link to commit>.

--8<---------------cut here---------------end--------------->8---

OK: this goes in the upcoming [PATCH] and related patch revision
process... :-D

[...]

>> Interesting!  This could also be done by a server post-receive hook, in
>> contrast to a remote service listening for UDP datagrams.
>
> The reason in my scheme why the more capable mumi CLI would be needed is
> because closed series would be inferred from commits Change-IDs rather
> than explicitly declared.

Yes I agree: a more capable mumi CLI is also needed in my scheme :-)

The "only" difference in my scheme is that we don't need an external
server listening for a UPD datagram, IMO a more capable version of our
current git hooks/post-receive script is better.

>>> What mumi does internally would be something like:
>>>
>>> a) Check in its database to establish the Change-Id <-> Issue # relation,
>>> if any.
>>>
>>> b) For each issue, if issue #'s known Change-Ids are all covered by the
>>> change-ids in the arguments, close it
>>
>> I think that b) is better suited for a git post-receive hook and not for
>> mumi triggered by a third service; as said above for sure such a script
>> needs mumi (CLI) to query the mumi (server) database.
>
> To clarify, the above should be a sequential process;

It was clear to me, thanks!

> with the Change-Id scheme, you don't have a direct mapping between a
> series and the Debbugs issue -- it needs to be figured out by checking
> in the Mumi database.

Yes, to be precise it needs to be figured out by a tool that is indexing
'Change-Id' via Xapian.

The preferred tool to be extended by the Guix project contributors is
mumi, obviously

... but a similar feature could also be provided by an enhanced version
of (the unofficial) guix-patches public-inbox, that uses Xapian queries
for searches [2], it "just" lacks indexing messages by 'Change-Id' (is
there a public-inbox CLI for searching and scripting purposes?!?)

[...]

> It could process the 'Fixes: #NNNNN' and other git trailers we choose to
> use as well, but what I had on mind was processing the *guix-patches*
> outstanding Debbugs issues based on the presence of unique Change-Ids in
> them.  It complements the other proposal as it could be useful for when
> a committer didn't specify the needed trailers and forgot to close the
> issue in *guix-patches*, for example.

Yes I think I get it :-)

To be cristal clear: I think that "the other proposal" (that is use
"Fixes:" and alike in commit msg to close the provided bug num) will be
**superseeded** when all the tools to manage (first of all: CLI query
tool) the 'Change-Id' preudo-header/footer :-D

>>> Since it'd be transparent and requires nothing from a committer, it'd
>>> provide value without having to document yet more processes.
>>
>> No, but we should however document the design of this new kind of
>> machinery, so we can always check that the implementation respects the
>> design and eventually redesign and refactor if needed.
>
> Yes, it should be summarily described at least in the documentation,
> with pointers to the source.
>
> Oof, that's getting long.

Wow! \O/

> To recap:
>
> We have two propositions in there:
>
> 1. A simple one that is declarative: new git trailers added to commit
> messages would convey actions to be done by the server-side hook.
>
> 2. A more complex one that would allow us to close *some* (not all) of
> the *guix-patches* issues automatically, relying on Change-Ids (and
> Mumi's ability to parse them) to infer which patch series saw all their
> commits merged already.
>
> I think both have value to be pursued, but 1. could be implemented first
> since it is simpler.

I think that finally we have a clear big picture.  Thanks!

As stated at the beginning of this message, I'm going to open bug
reports :D


Happy hacking! Gio'


[1] whatever "do" does mean: it could range from a "simple" search with
mumi (or something similar) by a human "bug reports gardener" to a full
fledged server side git hook to notify involved parties and/or
automatically close the related bug report.

[2] https://yhetil.org/guix-patches/_/text/help/

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]