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

2023-09-27 Thread Christopher Baines

Giovanni Biscuolo  writes:

> [[PGP Signed Part:Signature made by expired key D37D0EA7CECC3912 Giovanni 
> Biscuolo (Xelera) ]]
> Hi,
>
> Giovanni Biscuolo  writes:
>
> [...]
>
 The first thing we need is a server side git post-receive hook on
 Savannah, I've opened the sr#110928 support request:
 https://savannah.nongnu.org/support/index.php?110928
>>>
>>> It's something that the Savannah folks would need to maintain
>>> themselves, right?
>>
>> Forgot to mention that I'm pretty sure a post-receive server side hook
>> is already running (and maintained) for our guix.git repo on Savannah,
>> it's the one that sends notifications to guix-commits mailing list
>> https://lists.gnu.org/mailman/listinfo/guix-commits
>
> Regarding server side git hooks, I forgot to mention that on 2023-08-31
> a new commit-hook is available on Savannah (installation must be
> requested per-project):
>
> git post-receive UDP syndication
> https://savannah.gnu.org/news/?id=10508
>
>
> A new commit-hook is available to install for git repositories that will send 
> a single Datagram via UDP after each successful commit.  This can be useful 
> for continuous integration (CI) schemes and elsewise when a push driven model 
> is prefered to (e.g) regularly repolling upstream when changes may or may not 
> have occured. 
>
> To request installation please open a ticket with the Savannah Administration 
> project:
>
> [...]
>
> The (sh, GPLv3+) post-receive script source, detail on how the Datagram is 
> structured, and example "receiver" scripts (in perl) can be found here:
>
>   https://git.sr.ht/~mplscorwin/git-udp-syndicate
>
> Maybe this hook is useful for comminication with the QA service(s).

Not QA directly, but the Guix Data Service would benefit from supporting
this, as that could supplement or replace listening for emails to
determine when new revisions have been pushed.

I've added an entry to the TODO list in the README.


signature.asc
Description: PGP signature


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

2023-09-19 Thread Giovanni Biscuolo
Hi Vagrant,

Vagrant Cascadian  writes:

[...]

> This is why I think it is important for projects to have some sort of
> namespacing for this sort of thing; I am not really opinionated on the
> exact details, other than it not conflicting with Debian's terribly
> generic entirely un-namespaced historical "Closes:" syntax. :)

Noted and agreed!  At this point of discussion I also think there is
consensus (by the persons involved in this thread until now) to use a
namespaced URI (a full URL) for that upcoming feature (one of the two
discussed in the thread).

Thanks, Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-15 Thread Vagrant Cascadian
On 2023-09-14, Giovanni Biscuolo wrote:
> Maxim Cournoyer  writes:
>> I like the 'Closes: ' trailer idea; it's simple.  However, it'd need to
>> be something added locally, either the user typing it out (unlikely for
>> most contributors) or via some mumi wizardry (it's unlikely that all
>> users will use mumi), which means its usage (and value) would depend on
>> how motivated individuals are to learn these new tricks.
>
> I agree: the ratio, or better usecase, of my /trivial/ (in design, not
> in implementation) initial proposal [1] was to try to help committers
> closing bugs "in one go" by adding proper information to the commit
> message, e.g. "Closes: #N"

To be really, really clear, I am honestly pretty much fine with anything
except:

  Closes:.*#N

or

  Closes:.*N

These are already widely used in Debian, and their use was well
established before guix or even nix were even an inkling of an idea
wriggling around in any human brains.

Staying away from anything that uses any permutation of "close" would be
most appreciated. :)

(There may be some slightly more complicated variants; I think someone
posted a link to the regexp used earlier in the thread)

Since I push the entire relevent portions of upstream guix git history
when pushing changes for guix packaging in Debian, it would be a
significant bother for me if guix started using the same syntax, or
similar enough that a trivial typo might lead to something acting on the
wrong issue tracker...

This is why I think it is important for projects to have some sort of
namespacing for this sort of thing; I am not really opinionated on the
exact details, other than it not conflicting with Debian's terribly
generic entirely un-namespaced historical "Closes:" syntax. :)

live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-15 Thread Vagrant Cascadian
On 2023-09-15, Liliana Marie Prikler wrote:
> Am Donnerstag, dem 14.09.2023 um 15:51 -0700 schrieb Vagrant Cascadian:
>> On 2023-09-10, Liliana Marie Prikler wrote:
>> > Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant
>> > Cascadian:
>> > > I am much more comfortable with the "Fixes" convention of:
>> > > 
>> > >   Fixes: https://issues.guix.gnu.org/NNN
>> > I like the idea, but we should also consider the bugs.gnu.org
>> > address
>> > here as well as the convention of putting it into angular
>> > brackets.  In
>> > fact, I might even prefer it if the convention was
>> >   Fixes: Bug description 
>> > where bug description is a (possibly empty) name for the bug such
>> > as "Emacs hangs when I press a key" or something.
>> > 
>> > 
>> > As for when to send it, remember that we already send a bunch of
>> > mails to guix-comm...@gnu.org as our commit hook?  I think it
>> > shouldn't be too hard to search for the fixes line and send it to
>> > debbugs control.
>> 
>> Well, the complication gets to be ... which branch did it land in? in
>> master, it's fairly obvious... you can just mark it as
>> done/closed/etc. I guess with other branches it makes sense to mark
>> it with the "pending" or maybe some more specific usertag
>> "pending-in-BRANCH"?
> I don't think such a distinction is needed in most cases.  In fact, if
> it's about regular bugs, then a graft should likely hit master in case
> that an actual update is needed on another branch.  Other than that,
> it'd be silly to mark bugs specifically for e.g. "emacs-team" as still
> pending on the account of them not having hit master yet.

I guess I do not consider anything done until it lands in the master
branch, but obviously if it is committed in some branch, it is nice to
flag that somehow. "pending" seems appropriate up until it lands in
master.

Maybe marking by team or branch or whatnot is overkill, sure. Though it
would allow you could see at a glance which branch to look in without
diving into the whole history of the issue...

Of course, I will not make terrible loud noises if folks decide
otherwise. :)

live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-15 Thread Simon Tournier
Hi Giovanni,

Before commenting, let me repeat. ;-)

That’s said, I am not against the proposal.  I just have mixed
feelings and before deploying I strongly suggest to review if
the proposal covers the intent.


On Fri, 15 Sep 2023 at 09:16, Giovanni Biscuolo  wrote:

> Before commenting, just let me repeat that we are _copying_ the
> 'Change-Id' idea (and related possible implementation issues) from
> Gerrit:
>
> https://gerrit-review.googlesource.com/Documentation/user-changeid.html
>
> This means that Somewhere™ in our documentation we should start
> explaining that:
>
> --8<---cut here---start->8---
>
> Our code review system needs to identify commits that belong to the same
> review.  For instance, when a proposed patch needs to be modified to
> address the comments of code reviewers, a second version of that patch
> can be sent to guix-patc...@gnu.org.  Our code review system allows
> attaching those 2 commits to the same change, and relies upon a
> Change-Id line at the bottom of a commit message to do so.  With this
> Change-Id, our code review system can automatically associate a new
> version of a patch back to its original review, even across cherry-picks
> and rebases.
>
> --8<---cut here---end--->8---
>
> In other words, 'Change-Id' is /just/ metadata automatically added to
> help in code review **tracking**, specificcally helping "across
> cherry-picks and rebases" [1]
>
> Sorry if I'm repeating things probably already understood!

All this does not address my concern. :-)


>> 1. The hook must be installed.

[...]

> If this is stil not properly documented it will be fixed.

Maybe… and it will be another item in the already very long list of
steps to complete before contributing.  This is one of my concern: add
yet another thing to an already complicated process for contributing.


>> 2. The hook must not be in conflict with user configuration.
>
> I guess you mean not in conflict with other locally installed git hooks:
> since AFAIU we **already** have a locally installed git hook, this is
> already a requirement and this is something the user (contributor)
> should be aware of.

AFAIK, we have a pre-push hook.  This hook is only useful for the small
set of people who push the code.  And we can assume that this set of
people is skilled and are able to invest if something is unexpected.
Being Guix committer implies such commitment, IMHO.

Here, we are speaking for a Git hook applied to all.  That’s a different
situation, IMHO.

As an user and simple contributor, if I already have a global pre-commit
hook and then when I want to contribute to Guix, I potentially hit some
unexpected behaviour or even conflict, then as an user, either a. I give
up, drop and move my interest to something else than Guix, either b. I
spend some time for fixing this annoyance and that is pure friction (no
value for me and no value for the project).

> If this is stil not properly documented it will be fixed.

Yes, yet another thing to an already complicated process for
contributing.


>> 3. The generated Change-Id string can be mangled by some user unexpected
>>action.
>
> Contributors and committers should not delete or change and already
> existing 'Change-Id', this will be documented.

Yes, yet another thing to an already complicated process for
contributing.


>> Many things can rail out on user side.  For an example, base-commit is
>> almost appearing systematically in submitted patches almost 3 years
>> later.
>
> I don't understand how this could impact the addition of the
> patch-tracking metadata named 'Change-Id'
>
>> The patches of some submissions are badly formatted.  Etc.
>
> I don't understand what is the problem of having a 'Change-Id' (in
> commit messages) in badly formatted patch submissions.

In these both example, I just pointed that we already have some
potential frictions.  And here, another one is added.

What I know from my experience at work and from the small experience
backed by my reading of help-guix or some bug reports is that:
robustness is hard in an environment you do not control.  More action we
add in that uncontrolled environment and weaker the robustness becomes;
and weak robustness means friction.


Again, I am not saying the proposal is not worth – I am following the
discussion with interest. :-) I am just trying to ask if this proposal
will really fix an concrete annoyance compared to the relative
complexity it adds, and if it is the right balance between the part and
the counter-part.

Cheers,
simon



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

2023-09-15 Thread Giovanni Biscuolo
Hello Simon,

thank you for havig listed possible troubles.

Before commenting, just let me repeat that we are _copying_ the
'Change-Id' idea (and related possible implementation issues) from
Gerrit:

https://gerrit-review.googlesource.com/Documentation/user-changeid.html

This means that Somewhere™ in our documentation we should start
explaining that:

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

Our code review system needs to identify commits that belong to the same
review.  For instance, when a proposed patch needs to be modified to
address the comments of code reviewers, a second version of that patch
can be sent to guix-patc...@gnu.org.  Our code review system allows
attaching those 2 commits to the same change, and relies upon a
Change-Id line at the bottom of a commit message to do so.  With this
Change-Id, our code review system can automatically associate a new
version of a patch back to its original review, even across cherry-picks
and rebases.

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

In other words, 'Change-Id' is /just/ metadata automatically added to
help in code review **tracking**, specificcally helping "across
cherry-picks and rebases" [1]

Sorry if I'm repeating things probably already understood!

Simon Tournier  writes:

[...]

>> Please can you expand what troubles do you see in automatically adding
>> 'Change-Id:' using a hook-commit-msg like
>> https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
>> ?
>
> 1. The hook must be installed.

AFAIU a hook is already installed when configuring for contribution.

If this is stil not properly documented it will be fixed.

> 2. The hook must not be in conflict with user configuration.

I guess you mean not in conflict with other locally installed git hooks:
since AFAIU we **already** have a locally installed git hook, this is
already a requirement and this is something the user (contributor)
should be aware of.

If this is stil not properly documented it will be fixed.

> 3. The generated Change-Id string can be mangled by some user unexpected
>action.

Contributors and committers should not delete or change and already
existing 'Change-Id', this will be documented.

> Many things can rail out on user side.  For an example, base-commit is
> almost appearing systematically in submitted patches almost 3 years
> later.

I don't understand how this could impact the addition of the
patch-tracking metadata named 'Change-Id'

> The patches of some submissions are badly formatted.  Etc.

I don't understand what is the problem of having a 'Change-Id' (in
commit messages) in badly formatted patch submissions.

> Whatever the implementation, I am not convinced that the effort is worth
> the benefits.

OK, I'm sorry

> And I am not convinced it will help in closing the submissions when
> the patches have already been applied.

OK, I'm sorry

> That’s said, I am not against the proposal.  I just have mixed feelings
> and before deploying I strongly suggest to review if the proposal covers
> the intent.

OK, thank you for your suggestion.

Happy hacking! Gio'


[1] AFAIU 'Change-Id' can even track different versions of patches (that
by design are from commits in the same branch, properly rebased as
needed) sent by mistake via **different bug reports**, this also means
that different bug reports containing the same 'Change-Id' are _surely_
linked togheter.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-14 Thread Liliana Marie Prikler
Am Donnerstag, dem 14.09.2023 um 15:51 -0700 schrieb Vagrant Cascadian:
> On 2023-09-10, Liliana Marie Prikler wrote:
> > Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant
> > Cascadian:
> > > I am much more comfortable with the "Fixes" convention of:
> > > 
> > >   Fixes: https://issues.guix.gnu.org/NNN
> > I like the idea, but we should also consider the bugs.gnu.org
> > address
> > here as well as the convention of putting it into angular
> > brackets.  In
> > fact, I might even prefer it if the convention was
> >   Fixes: Bug description 
> > where bug description is a (possibly empty) name for the bug such
> > as "Emacs hangs when I press a key" or something.
> > 
> > 
> > As for when to send it, remember that we already send a bunch of
> > mails to guix-comm...@gnu.org as our commit hook?  I think it
> > shouldn't be too hard to search for the fixes line and send it to
> > debbugs control.
> 
> Well, the complication gets to be ... which branch did it land in? in
> master, it's fairly obvious... you can just mark it as
> done/closed/etc. I guess with other branches it makes sense to mark
> it with the "pending" or maybe some more specific usertag
> "pending-in-BRANCH"?
I don't think such a distinction is needed in most cases.  In fact, if
it's about regular bugs, then a graft should likely hit master in case
that an actual update is needed on another branch.  Other than that,
it'd be silly to mark bugs specifically for e.g. "emacs-team" as still
pending on the account of them not having hit master yet.

Cheers



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

2023-09-14 Thread Vagrant Cascadian
On 2023-09-10, Liliana Marie Prikler wrote:
> Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant Cascadian:
>> I am much more comfortable with the "Fixes" convention of:
>> 
>>   Fixes: https://issues.guix.gnu.org/NNN
> I like the idea, but we should also consider the bugs.gnu.org address
> here as well as the convention of putting it into angular brackets.  In
> fact, I might even prefer it if the convention was
>   Fixes: Bug description 
> where bug description is a (possibly empty) name for the bug such as
> "Emacs hangs when I press a key" or something.
>
>
> As for when to send it, remember that we already send a bunch of mails
> to guix-comm...@gnu.org as our commit hook?  I think it shouldn't be
> too hard to search for the fixes line and send it to debbugs control.

Well, the complication gets to be ... which branch did it land in? in
master, it's fairly obvious... you can just mark it as
done/closed/etc. I guess with other branches it makes sense to mark it
with the "pending" or maybe some more specific usertag
"pending-in-BRANCH"?

live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-14 Thread Maxim Cournoyer
Hello,

Le 14 septembre 2023 12:30:41 GMT-04:00, Vagrant Cascadian  
a écrit :

[…]

>> I doubt the Change-Id idea would help much closing *bugs* on the
>> bug-guix tracker, but I'd expect it to be useful to close already merged
>> (but forgotten on the guix-patches tracker) *patches*.
>
>Well, all of the "bugs" I was referring to were submitted patches
>tracked at debbugs.gnu.org via the issues.guix.gnu.org
>frontend... weather they were submitted via guix-patches@ or bug-guix@
>or n...@debbugs.gnu.org... :)
>
>They are all "bugs" to me, though "issues" seems a bit more neutral in
>tone and I like it better, but new packages or package updates are just
>generally wishlist bugs/issues and not inherrently something broken,
>though they may sometimes fix things or be related to fixes...

Let's call these patch series to avoid confusion :-)

>
>> The Change-Ids would be added automatically without the user having to
>> install anything, so from the time it's deployed we'd have a means to
>> identify which patch series were all merged.
>
>Well, they would still have to install the commit hook, or did I miss
>something?

Yes! Hooks can be configured upon building the Guix checkout, the same way its 
now done for our pre-push hook :-)

Maxim



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

2023-09-14 Thread Liliana Marie Prikler
Am Donnerstag, dem 14.09.2023 um 11:42 +0200 schrieb Giovanni Biscuolo:
> 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?
On "Fixes:" vs. "Applies:" etc: the point of having more keywords is
that "Fixes:" implies a bug, whereas more general wording is preferable
when pushing a patch that simply updates a package.  I just wrote some
example wordings and wanted all of us to agree on the one that makes
"the most sense"; alas it got misinterpreted.

> 
> > Maybe if Vagrant put something like:
> > 
> > Fixes:  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:
> " is OK for your usecase?
I actually prefer cool URLs myself, my format was 
"Fixes: [optional description] "

> > 
> 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
Well I still very much prefer human readable footers, but I'm perhaps a
little lonely in this debate.

Cheers
> > > 




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

2023-09-14 Thread Simon Tournier
Hi,

On Thu, 14 Sep 2023 at 12:27, Giovanni Biscuolo  wrote:

> Please can you expand what troubles do you see in automatically adding
> 'Change-Id:' using a hook-commit-msg like
> https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
> ?

1. The hook must be installed.
2. The hook must not be in conflict with user configuration.
3. The generated Change-Id string can be mangled by some user unexpected
   action.
4. etc.

Many things can rail out on user side.  For an example, base-commit is
almost appearing systematically in submitted patches almost 3 years
later.  The patches of some submissions are badly formatted.  Etc.

Whatever the implementation, I am not convinced that the effort is worth
the benefits.  And I am not convinced it will help in closing the
submissions when the patches have already been applied.

That’s said, I am not against the proposal.  I just have mixed feelings
and before deploying I strongly suggest to review if the proposal covers
the intent.

Cheers,
simon



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

2023-09-14 Thread Vagrant Cascadian
On 2023-09-13, Maxim Cournoyer wrote:
> Vagrant Cascadian  writes:
>> On 2023-09-09, Maxim Cournoyer wrote:
>>> The Change-Id stays the same unless you manually edit it out of your
>>> commit message when amending / rebasing, so the commit hash may change
>>> while the Change-Id stays the same.  So you can rebase your feature
>>> branch on master and share a v2, whose existing commits will have the
>>> same Change-Ids (newly added commits would get their own Change-Id
>>> trailer).
>>
>> Ok, that makes some sense.
>>
>> Although the majority of bugs in the cleanup work I did were actually
>> filed by someone else entirely... so the Change-Id will not help with
>> those. Not a reason not to implement it, but not consistent with some of
>> the triggers of this thread. :)
>
> I doubt the Change-Id idea would help much closing *bugs* on the
> bug-guix tracker, but I'd expect it to be useful to close already merged
> (but forgotten on the guix-patches tracker) *patches*.

Well, all of the "bugs" I was referring to were submitted patches
tracked at debbugs.gnu.org via the issues.guix.gnu.org
frontend... weather they were submitted via guix-patches@ or bug-guix@
or n...@debbugs.gnu.org... :)

They are all "bugs" to me, though "issues" seems a bit more neutral in
tone and I like it better, but new packages or package updates are just
generally wishlist bugs/issues and not inherrently something broken,
though they may sometimes fix things or be related to fixes...


> The Change-Ids would be added automatically without the user having to
> install anything, so from the time it's deployed we'd have a means to
> identify which patch series were all merged.

Well, they would still have to install the commit hook, or did I miss
something?


live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-14 Thread Giovanni Biscuolo
Maxim Cournoyer  writes:

[...]

> I like the 'Closes: ' trailer idea; it's simple.  However, it'd need to
> be something added locally, either the user typing it out (unlikely for
> most contributors) or via some mumi wizardry (it's unlikely that all
> users will use mumi), which means its usage (and value) would depend on
> how motivated individuals are to learn these new tricks.

I agree: the ratio, or better usecase, of my /trivial/ (in design, not
in implementation) initial proposal [1] was to try to help committers
closing bugs "in one go" by adding proper information to the commit
message, e.g. "Closes: #N"

It was _not_ intended for contributors, also because they could _not_
know that **specific** patch in a patch series will really close a
**whole** bug report: that's only a judgement of the committer.

Also, my ratio was influenced by my misunderstanding of a series of bug
closing actions performed by Vagrant (see [1] for details): the problem
in the majority (all?) of those cases was **not** that the committer
simply forgot to close the related bug report /but/ that bug reports
containing (different) patches for the _same_ package were not linked
each other: the solution to this class of problems in obviously not
"Automatically close bug report when a patch is committed", it's
something else [2]

> On the other hands, having Change-Ids added by a pre-commit hook
> automatically would means the user doesn't need to do anything special
> other than using git, and we could still infer useful information at any
> time (in a server hook, or as a batch process).
>
> For this reason, I think we could have both (why not?  Change-Ids by
> themselves provide some value already -- traceability between our git
> history and guix-patches).

I agree: just having 'Change-Ids' alone already provide some added
value, even if we still miss the tooling (server side git hook, batch
processing

Thanks!  Gio'


[1] id:8734zrn1sc@xelera.eu 
https://yhetil.org/guix/8734zrn1sc@xelera.eu/

[2] id:87msxyfhmv@xelera.eu https://yhetil.org/guix/87msxyfhmv@xelera.eu

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-14 Thread Giovanni Biscuolo
Simon Tournier  writes:

[...]

>>  maybe ChangeIds really trump the explicit tags proposed by Giovanni
>> or myself here.  Whether that justifies the cognitive overhead of
>> juggling them around on every submission remains to be shown or
>> disproven.
>
> I agree.  I am not convinced by the benefits and I already see some
> troubles.

Please can you expand what troubles do you see in automatically adding
'Change-Id:' using a hook-commit-msg like
https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
?

Thanks, Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-14 Thread Giovanni Biscuolo
Hi Liliana

Liliana Marie Prikler  writes:

> Am Mittwoch, dem 13.09.2023 um 11:27 -0400 schrieb Maxim Cournoyer:

[...]

> I do wonder how the ChangeId would work in practice.

It's a «tag to track commits across cherry-picks and rebases.»

It is used by Gerrit to identify commits that belong to the same review:
https://gerrit-review.googlesource.com/Documentation/user-changeid.html

We could use it for the same purpose and instead of building a web
application for code review, "simply" count that all 'Change-Id's in a
patchset have been pushed to the Guix official repo to declare the
related bug report closed.

> Since it's not really assigned by the committer, it would have to be
> generated "on the fly" and attached to the mail in between

Not to the mail, to the commit msg! [1]

> which could result in all kinds of nasty behaviour like unstable Ids
> or duplicated ones.

No, modulo hook script bugs obviously.

> Also, if we can automate this for ChangeIds, we could also automate
> this for patch-sets – the last patch in the series just gets the
> Closes: tag added by mumi.  

The idea is that, but we don't need to add "Closes" to the commit msg
(via post-receive hook), we "just" need the hook to send an email to
-done on behalf of the committer (the committer, not the
contributor).

> Furthermore, I'm not convinced that it would ease the issue of
> forgotten bugs as you can't really apply them to the past.

No, this 'Change-Id' is not intended for past bug reports since we
**must not** rewrite past commits _because_ commit messages are
/embedded/ in commit objects.

...but for this purpose we could use git-notes, **if** wanted:
https://git-scm.com/docs/git-notes :-D

> So the practical use is limited to the case where you intentionally
> cherry- pick this or that commit from a series.

No: the practical use is that for each guix-patch bug report we can
count how many [PATCH]es are left to be committed and act accordigly,
for example notify all involved parties (contributor, committer,
'X-Debbugs-CC's) that N/M patches from the series are still to be merged
upstream... or close the bug report if zero patches are left.

> How we want to deal with that case could be a discussion in its own
> right, and maybe ChangeIds really trump the explicit tags proposed by
> Giovanni or myself here.  Whether that justifies the cognitive
> overhead of juggling them around on every submission remains to be
> shown or disproven.

There will be no additional cognitive overhead for contributors since
'Change-Id' will be automatically managed, they can simply ignore it.

> Beyond the scope of the discussion so far, it also doesn't help us with
> duplicate or superseded patches (e.g. two series on the mailing list
> propose a similar change, because one of them has already been
> forgotten).

No, IMO there is **no** solution to this problems other than "triaging"
(id:87msxyfhmv@xelera.eu
https://yhetil.org/guix/87msxyfhmv@xelera.eu/)

> Again, the explicit close tags would allow this case to be
> handled in an interpretable fashion.  In both cases, we do however also
> introduce the potential for incorrect tagging, which then needs to be
> resolved manually (more or less a non-issue, as it's the status quo).

There is no potential of incorret tagging when using a hook-commit-msg
[1] to add 'Change-Id'.

For the other method discussed here, there is no way to avoid users
mistyping 'Closes:' pseuto-headers in their commit messages: if mistuped
they will be ignored :-(


Cheeers, Gio'


[1] 
https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-14 Thread Giovanni Biscuolo
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  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]:
>>
>>
>>So... 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:  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:
" is OK for your usecase?

[...]

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

Thinking twice about this point, now I see that using the URL is
**much** better than , 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 
>> Reply-To:  <>
>> To: cont...@debbugs.gnu.org
>>
>>
>> and this body:
>>
>>
>> package guix
>> close  []
>> 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 -d...@debbugs.gnu.org?

Like:


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

2023-09-14 Thread Andreas Enge
Hello,

Am Wed, Sep 13, 2023 at 09:14:52PM +0200 schrieb Liliana Marie Prikler:
> I do wonder how the ChangeId would work in practice.  Since it's not
> really assigned by the committer, it would have to be generated "on the
> fly" and attached to the mail in between, which could result in all
> kinds of nasty behaviour like unstable Ids or duplicated ones.

this one would be easy to solve: it could just be the hash of something.
For instance, it could actually be the commit "number" itself of the
commit when it is first created.

Andreas




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

2023-09-13 Thread Maxim Cournoyer
Hi,

Simon Tournier  writes:

> Hi,
>
> On Wed, 13 Sep 2023 at 21:14, Liliana Marie Prikler 
>  wrote:
>
>> I do wonder how the ChangeId would work in practice.  Since it's not
>> really assigned by the committer, it would have to be generated "on the
>> fly" and attached to the mail in between, which could result in all
>> kinds of nasty behaviour like unstable Ids or duplicated ones.  Also,
>> if we can automate this for ChangeIds, we could also automate this for
>> patch-sets – the last patch in the series just gets the Closes: tag
>> added by mumi.
>
> I think it would work using some pre-commit hook.  When one commits
> their change, this commit is run and it can pre-fill the commit
> message.  Well, that’s how I have understood the thread.

Yes; exactly like how it's done in Gerrit, if you've ever used that
(we'd reuse their hook).  It'd be enabled out-of-the-box so it'd be
transparent to users.

>> Furthermore, I'm not convinced that it would ease the issue of
>> forgotten bugs as you can't really apply them to the past.  So the
>> practical use is limited to the case where you intentionally cherry-
>> pick this or that commit from a series.  How we want to deal with that
>> case could be a discussion in its own right, and maybe ChangeIds really
>> trump the explicit tags proposed by Giovanni or myself here.  Whether
>> that justifies the cognitive overhead of juggling them around on every
>> submission remains to be shown or disproven.

I like the 'Closes: ' trailer idea; it's simple.  However, it'd need to
be something added locally, either the user typing it out (unlikely for
most contributors) or via some mumi wizardry (it's unlikely that all
users will use mumi), which means its usage (and value) would depend on
how motivated individuals are to learn these new tricks.

On the other hands, having Change-Ids added by a pre-commit hook
automatically would means the user doesn't need to do anything special
other than using git, and we could still infer useful information at any
time (in a server hook, or as a batch process).

For this reason, I think we could have both (why not?  Change-Ids by
themselves provide some value already -- traceability between our git
history and guix-patches).

-- 
Thanks,
Maxim



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

2023-09-13 Thread Simon Tournier
Hi,

On Wed, 13 Sep 2023 at 21:14, Liliana Marie Prikler  
wrote:

> I do wonder how the ChangeId would work in practice.  Since it's not
> really assigned by the committer, it would have to be generated "on the
> fly" and attached to the mail in between, which could result in all
> kinds of nasty behaviour like unstable Ids or duplicated ones.  Also,
> if we can automate this for ChangeIds, we could also automate this for
> patch-sets – the last patch in the series just gets the Closes: tag
> added by mumi.

I think it would work using some pre-commit hook.  When one commits
their change, this commit is run and it can pre-fill the commit
message.  Well, that’s how I have understood the thread.


> Furthermore, I'm not convinced that it would ease the issue of
> forgotten bugs as you can't really apply them to the past.  So the
> practical use is limited to the case where you intentionally cherry-
> pick this or that commit from a series.  How we want to deal with that
> case could be a discussion in its own right, and maybe ChangeIds really
> trump the explicit tags proposed by Giovanni or myself here.  Whether
> that justifies the cognitive overhead of juggling them around on every
> submission remains to be shown or disproven.

I agree.  I am not convinced by the benefits and I already see some
troubles.

Cheers,
simon



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

2023-09-13 Thread Liliana Marie Prikler
Am Mittwoch, dem 13.09.2023 um 11:27 -0400 schrieb Maxim Cournoyer:
> For just closing cross-referenced bugs, I agree.  For closing
> forgotten, already merged issues on guix-patches we'd need the
> Change-Id and a tool able to map Change-Ids -> issue number (mumi is
> in the best place to do so).
> 
> It's been a hard discussion to follow, but I think we're coming to
> some understanding that we are discussing two different schemes that
> could be both implemented to provide different benefits, right?
I do wonder how the ChangeId would work in practice.  Since it's not
really assigned by the committer, it would have to be generated "on the
fly" and attached to the mail in between, which could result in all
kinds of nasty behaviour like unstable Ids or duplicated ones.  Also,
if we can automate this for ChangeIds, we could also automate this for
patch-sets – the last patch in the series just gets the Closes: tag
added by mumi.  

Furthermore, I'm not convinced that it would ease the issue of
forgotten bugs as you can't really apply them to the past.  So the
practical use is limited to the case where you intentionally cherry-
pick this or that commit from a series.  How we want to deal with that
case could be a discussion in its own right, and maybe ChangeIds really
trump the explicit tags proposed by Giovanni or myself here.  Whether
that justifies the cognitive overhead of juggling them around on every
submission remains to be shown or disproven.

Beyond the scope of the discussion so far, it also doesn't help us with
duplicate or superseded patches (e.g. two series on the mailing list
propose a similar change, because one of them has already been
forgotten).  Again, the explicit close tags would allow this case to be
handled in an interpretable fashion.  In both cases, we do however also
introduce the potential for incorrect tagging, which then needs to be
resolved manually (more or less a non-issue, as it's the status quo).

Cheers



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

2023-09-13 Thread Maxim Cournoyer
Hi,

Vagrant Cascadian  writes:

> On 2023-09-09, Maxim Cournoyer wrote:
>> Vagrant Cascadian  writes:
 Did you see my message about integrating a commit-hook similar to what
 Gerrit uses?  It produces unique ID such as:

 --8<---cut here---start->8---
 Change-Id: I9b86781869d80eda347659f0c009b8dfe09bdfd0
 --8<---cut here---end--->8---
> ...
>>> That seems like it would only work if the patch was identical, as
>>> opposed to a slightly rebased patch on top of newer patches on master?
>>>
>>> How can you correlate Change-Id to a patch in the tracker?
>>
>> The Change-Id stays the same unless you manually edit it out of your
>> commit message when amending / rebasing, so the commit hash may change
>> while the Change-Id stays the same.  So you can rebase your feature
>> branch on master and share a v2, whose existing commits will have the
>> same Change-Ids (newly added commits would get their own Change-Id
>> trailer).
>
> Ok, that makes some sense.
>
> Although the majority of bugs in the cleanup work I did were actually
> filed by someone else entirely... so the Change-Id will not help with
> those. Not a reason not to implement it, but not consistent with some of
> the triggers of this thread. :)

I doubt the Change-Id idea would help much closing *bugs* on the
bug-guix tracker, but I'd expect it to be useful to close already merged
(but forgotten on the guix-patches tracker) *patches*.

The Change-Ids would be added automatically without the user having to
install anything, so from the time it's deployed we'd have a means to
identify which patch series were all merged.

-- 
Thanks,
Maxim



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

2023-09-13 Thread Maxim Cournoyer
Hi Liliana,

Liliana Marie Prikler  writes:

> Hi Maxim,
>
> Am Montag, dem 11.09.2023 um 16:41 -0400 schrieb Maxim Cournoyer:
>> [...]
>> Perhaps both approach[es] could be combined.  I still see value in a
>> general scheme to automate closing applied series that linger on in
>> Debbugs.
>> 
>> [0] 
>> https://lists.gnu.org/archive/html/guix-devel/2023-09/msg00138.html
>> 
>> Change-Ids would also add the benefit that any commit found in Git
>> could easily be traced back to their submission on the guix-patches
>> or guix trackers or vice-versa (git log --grep='Change-Id='), as
>> noted by Giovanni.
> The thing is, we're discussing the same basic workflow (which you lay
> out below), just different kinds of metadata that we'd have to attach
> to our commits.  IIUC ChangeIds need to actually be carried around by
> the committers as they e.g. rewrite patches (rebasing, squashing, what
> have you), and they're basically opaque hashes so I don't see the
> benefit to the reader.  (I think you might be arguing that the benefit
> is uniqueness, but I'm not sure if I ought to buy that.)

Correct; the Change-Ids must be preserved when rebasing ((*if* they were
already published -- otherwise it doesn't matter)), and for the human
reader they're mostly noise.  Since it can be automated though, from the
day it's added all the commits would have one and they would become a
valuable unique key to cross-reference between what's in master and what
was reviewed in guix-patches, for example.

> Meanwhile "Fixes: [whatever notation]" also needs to carried around,
> sure, but at the same time provides semantics by pointing to a (known)
> bug report.  Now again, I personally prefer cool URLs here, but that's
> a bike we can shed however we want.

That's nice, and as I wrote in my previous reply to Giovanni, I think
both schemes have their place.

>> The process could go like this:
>> 
>> 1. commits of a series pushed to master
>> 2. Savannah sends datagram to a remote machine to trigger the
>> post-commit job, with the newly pushed commits 'Change-Id' values (a
>> list of them).
>> 3. The remote machine runs something like 'mumi close-issues [change-
>> id-1 change-id-2 ...]'
> Yeah, I think we basically agree on the 1 and 2 here, but I don't think
> we have to really implement 3.  IMHO we could do something simpler for
> all parties by just carrying the bug number around (in whichever form),
> which we do for some of our pre-ChangeLog explanations already.

For just closing cross-referenced bugs, I agree.  For closing forgotten,
already merged issues on guix-patches we'd need the Change-Id and a tool
able to map Change-Ids -> issue number (mumi is in the best place to do
so).

It's been a hard discussion to follow, but I think we're coming to some
understanding that we are discussing two different schemes that could be
both implemented to provide different benefits, right?

-- 
Thanks,
Maxim



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

2023-09-13 Thread Maxim Cournoyer
Hi Giovanni,

Giovanni Biscuolo  writes:

[...]

 Liliana Marie Prikler  writes:
>
 > WDYT about the following
 >   Applies: [patch] 
 >   Closes: [patch] 
 >   Resolves: [patch] 
 >   Done: [patch] 

[...]

> 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 both 'Closes and 'Fixes' would be implement via the
n-d...@debbugs.gnu.org special email sent to the control server, but
they convey a different idea to us humans.  I agree.

[...]

>> 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!  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?
What I described here was a general process by which we could close
*patches* series that were forgotten in an 'open' state.

> This is something we sould consider, but it's another topic: how to
> effectively link commits to #bug-num (I guess we already talked about
> this in some other thread)
>
>> I'm not sure what [patch] or namespace add (is it for a fancy URL?), so
>> I'd drop them.
>
> I'll try to recap, sorry for the repetitions!
>
> Namespace has been assumed as part of the proposed URI to try address
> Vagrant's concerns [1]:
>
>
>So... 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:  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.  And if we accept just 'Fixes: #N', it
should be documented that these are strictly intended to refer to Guix
issues, not that of other projects.  I don't think we'll be able to make
this error proof, so we should instead aim to make it convenient for the
main use cases, which is to refer to Guix issues.  Does that make sense?

> Fixes: [optional bug description] 
>
> (here, URI is :#)
>
> ...but then you, Maxim, suggested [3] this form:
>
> Fixes: bug#65738 (java-ts-mode tests)

Note that we can stick with the  URL and
achieve the same result with some extra config (see: bug#65883).

 If so, that's adding rather than reducing friction, and I'm not sure
 it'd gain much traction.  The way I see it, it needs to happen
 automatically.
>>> I mean, the way I imagine is that you type this as part of your message
>>> and then debbugs would do the work of closing the bug.  In short, "git
>>> push" saves you the work of writing a mail because there's a hook for
>>> it.
>
> I guess all of us are looking for this very same thing: a server side
> web hook that automatically closes bugs (via email) when committers
> pushing "instructs" it to do so.
>
> The automatic email message will be sent to our "bug control and
> manipulation server" [5], with this header:
>
>
> From: GNU Guix git hook 
> Reply-To:  <>
> To: cont...@debbugs.gnu.org
>
>
> and this body:
>
>
> package guix
> close  []
> 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.

> A notification is sent to the user who reported the bug, but (in
> contrast to mailing bugnumber-done) the text of the mail which caused
> the bug to be closed is not included in that notification.
>
> If you supply a fixed-version, the bug tracking system will note that
> the bug was fixed in that version of the package.
>
> Last but not least, the very fact that "GNU Guix git hook" have closed
> the bug report is tracked and showed in the bug report history, as any
> other action made via email using the Debbugs control 

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

2023-09-13 Thread Giovanni Biscuolo
Hi Liliana and Maxim,

Liliana Marie Prikler  writes:

[...]

> The thing is, we're discussing the same basic workflow

No, we are discussing two (complementary) workflows:

1. the one I suggested to add a "footer-metadata-field" named
Fix/Fixes/Close/Closes/whatever that will allow people pushing to the
Guix official repo to _add_ the information that the (already installed,
to be enhanced) server side git post-receive hook should also close one
or more bug reports; that "metadata-footer" should be "manually" added
to the commit message before pushing, the commit must be _amended_ (git
commit --amend).

2. the one suggested by Maxim (bringed by Gerrit) to _automatically_ add
a "footer-metadata-field" named 'Change-Id' that will allow "a
machinery" (IMO it should be the currently installed hook, enhanced) to
_automaticcally_ close bug reports when all 'Change-Id's contained in a
bug report have been pushed to the official Guix repo.

This is my understanding of what we are discussing here: did I miss
something?

> (which you lay out below), just different kinds of metadata that we'd
> have to attach to our commits.

Thay are different because they serve different needs.

> IIUC ChangeIds need to actually be carried around by the committers as
> they e.g. rewrite patches (rebasing, squashing, what have you)

Since 'Change-Id' is automaticcaly generated by a _local_ git hook upon
committing and left unchanged if already present, the only precaution
the committer should apply is to preserve it when rebasing in case the
person needs to send a new version of the patch. 

> and they're basically opaque hashes so I don't see the benefit to the
> reader.

The benefit are not for the reader but for "the machinery" to be able to
compute when a patch set is completely pushed to the Guix official repo,
this also means that the related bug repo (related to the patch set) can
be happily automatically closed.  No?

> (I think you might be arguing that the benefit is uniqueness, but I'm
> not sure if I ought to buy that.)

The benefit is that 'Change-Id' is autogererated as unique, kept between
rebases (with some pracaution by the _local_ committer) thus is useful
to compute the completion of each patch contained in a bug repo (of
class [PATCH]).

Obviously all of this should be clearly documented, so everyone will
understand how it works.

[...]

>> In case it couldn't close an issue, it could send a notification to
>> the submitter: "hey, I've seen some commits of series  landing to
>> master, but not all of the commits appears to have been pushed,
>> please check"
> I'm not sure how common this case is

Don't know the cardinality, but I guess is a /very/ useful usecase for
people who have committ access to the official Guix repo... at least for
Maxim :-)

Anyway, I think the main use case for this _second_ way of automatically
closing related bugs (the one based upon 'Change-Id') when all [PATCH]es
are pushed is very useful for all people with commit access to the Guix
repo, a sort of "push and (almost) forget".

[...]

Ciao, Gio'.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-12 Thread Vagrant Cascadian
On 2023-09-09, Maxim Cournoyer wrote:
> Vagrant Cascadian  writes:
>>> Did you see my message about integrating a commit-hook similar to what
>>> Gerrit uses?  It produces unique ID such as:
>>>
>>> --8<---cut here---start->8---
>>> Change-Id: I9b86781869d80eda347659f0c009b8dfe09bdfd0
>>> --8<---cut here---end--->8---
...
>> That seems like it would only work if the patch was identical, as
>> opposed to a slightly rebased patch on top of newer patches on master?
>>
>> How can you correlate Change-Id to a patch in the tracker?
>
> The Change-Id stays the same unless you manually edit it out of your
> commit message when amending / rebasing, so the commit hash may change
> while the Change-Id stays the same.  So you can rebase your feature
> branch on master and share a v2, whose existing commits will have the
> same Change-Ids (newly added commits would get their own Change-Id
> trailer).

Ok, that makes some sense.

Although the majority of bugs in the cleanup work I did were actually
filed by someone else entirely... so the Change-Id will not help with
those. Not a reason not to implement it, but not consistent with some of
the triggers of this thread. :)

live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-12 Thread Liliana Marie Prikler
Hi Maxim,

Am Montag, dem 11.09.2023 um 16:41 -0400 schrieb Maxim Cournoyer:
> [...]
> Perhaps both approach[es] could be combined.  I still see value in a
> general scheme to automate closing applied series that linger on in
> Debbugs.
> 
> [0] 
> https://lists.gnu.org/archive/html/guix-devel/2023-09/msg00138.html
> 
> Change-Ids would also add the benefit that any commit found in Git
> could easily be traced back to their submission on the guix-patches
> or guix trackers or vice-versa (git log --grep='Change-Id='), as
> noted by Giovanni.
The thing is, we're discussing the same basic workflow (which you lay
out below), just different kinds of metadata that we'd have to attach
to our commits.  IIUC ChangeIds need to actually be carried around by
the committers as they e.g. rewrite patches (rebasing, squashing, what
have you), and they're basically opaque hashes so I don't see the
benefit to the reader.  (I think you might be arguing that the benefit
is uniqueness, but I'm not sure if I ought to buy that.)

Meanwhile "Fixes: [whatever notation]" also needs to carried around,
sure, but at the same time provides semantics by pointing to a (known)
bug report.  Now again, I personally prefer cool URLs here, but that's
a bike we can shed however we want.

> The process could go like this:
> 
> 1. commits of a series pushed to master
> 2. Savannah sends datagram to a remote machine to trigger the
> post-commit job, with the newly pushed commits 'Change-Id' values (a
> list of them).
> 3. The remote machine runs something like 'mumi close-issues [change-
> id-1 change-id-2 ...]'
Yeah, I think we basically agree on the 1 and 2 here, but I don't think
we have to really implement 3.  IMHO we could do something simpler for
all parties by just carrying the bug number around (in whichever form),
which we do for some of our pre-ChangeLog explanations already.
 
> In case it couldn't close an issue, it could send a notification to
> the submitter: "hey, I've seen some commits of series  landing to
> master, but not all of the commits appears to have been pushed,
> please check"
I'm not sure how common this case is, but if it's needed we could add
"Part-of: [same stuff as for fixes]" which once parsed by Debbugs/Mumi
would send that notification.


Cheers



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

2023-09-12 Thread Giovanni Biscuolo
Hi Maxim and Liliana,

Maxim Cournoyer  writes:

>>> Liliana Marie Prikler  writes:

>>> > WDYT about the following
>>> >   Applies: [patch] 
>>> >   Closes: [patch] 
>>> >   Resolves: [patch] 
>>> >   Done: [patch] 

[...]

>> I'm just asking which wording you prefer.  For the tracker, they'd mean
>> the same as "Fixes", but fixes imho sounds like a bug,

But we actually have to close/fix/resolve/mark-as-done a bug
(report), no?  :-D

>> which "Update Emacs to 29.2" isn't.  Thus, something with a more
>> neutral meaning like "Resolves" might apply better here.

IMO this is just an implementation "detail", although an important one.

Keywords are _just_ labels, the semantics is defined elsewhere (even
when it's "implicit") and the semantics establish the behaviour we
intend to implement via our algorithm we use to (re)program the git hook
in order to automatically close a bug report when the committer pushes
all needed patches in a bug of class "PATCH" or find it's appropriate to
mark it as done when committing another class of bug (sorry for being so
convoluted :-O )

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.

This means that a committer can write for example "RESOLVES:" and the
meaning is the same as "do:" or "Fixes:".

IMO the "superficial" semantic of the keyword apply/applies is ambiguos:
does this mean:

1. the commit is part of a series of patches tracked in #bug-num thus
the rest of the series is still to be worked upon (please don't close
the bug report)

or

2. the commit is the last one of all the patches tracked in #bug-num
thus no other patch is left to be committed (please close the bug
report)

WDYT?

> 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?

This is something we sould consider, but it's another topic: how to
effectively link commits to #bug-num (I guess we already talked about
this in some other thread)

> I'm not sure what [patch] or namespace add (is it for a fancy URL?), so
> I'd drop them.

I'll try to recap, sorry for the repetitions!

Namespace has been assumed as part of the proposed URI to try address
Vagrant's concerns [1]:

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

   So... 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.

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

Here we have a use case in which a Guix committer is also committer of
other projects using a similar git hook, with (a subset of) the _very
same_ keywords we choose for our hook.

That's why I proposed [2] a namespaced URI, possibly not including the
URL:

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

Fixes: [optional bug description] 

--8<---cut here---end--->8---
(here, URI is :#)

...but then you, Maxim, suggested [3] this form:

--8<---cut here---start->8---
Fixes: bug#65738 (java-ts-mode tests)
--8<---cut here---end--->8---
(here, URI is bug#8---

: bug#@ ()

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

I'm an Emacs user also and when I enable bug-reference-mode in this
message buffer I still see the "bug#" part of the string:

bug#65738@guix

is shown as an URL (pointing nowhere since I still did not configure my
Emacs properly)

Maxim: do you think my proposal could work also for Emacs
bug-reference-mode "machinery"?

And everyone: do you think that the above proposal for an "Emacs
compatible" namestaced URI could be fine for all considered usercases?

>>> If so, that's adding rather than reducing friction, and I'm not sure
>>> it'd gain much traction.  The way I see it, it needs to happen
>>> automatically.
>> I mean, the way I imagine is that you type this as part of your message
>> and then debbugs would do the work of closing the bug.  In short, "git
>> push" saves you the work of writing a mail because there's a hook for
>> it.

I guess all of us are looking for this very same thing: a server side
web hook that automatically closes bugs (via 

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

2023-09-11 Thread Maxim Cournoyer
Hi Liliana,

Liliana Marie Prikler  writes:

> Am Montag, dem 11.09.2023 um 14:36 -0400 schrieb Maxim Cournoyer:
>> Hi,
>>
>> Liliana Marie Prikler  writes:
>>
>> [...]
>>
>> > Maybe make that bug-guix so that it matches with the mailing list
>> > name,
>> > though we also need a wording for when a patch is not a bug, e.g. a
>> > simple package upgrade.
>> >
>> > WDYT about the following
>> >   Applies: [patch] 
>> >   Closes: [patch] 
>> >   Resolves: [patch] 
>> >   Done: [patch] 
>>
>> I don't follow; why do we need 'Applies' ?  Why do we need a
>> 'namespace'.  Are these things the user would need to manually know
>> and enter themselves in their commit messages?
> I'm just asking which wording you prefer.  For the tracker, they'd mean
> the same as "Fixes", but fixes imho sounds like a bug, which "Update
> Emacs to 29.2" isn't.  Thus, something with a more neutral meaning like
> "Resolves" might apply better here.

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---

I'm not sure what [patch] or namespace add (is it for a fancy URL?), so
I'd drop them.

>> If so, that's adding rather than reducing friction, and I'm not sure
>> it'd gain much traction.  The way I see it, it needs to happen
>> automatically.
> I mean, the way I imagine is that you type this as part of your message
> and then debbugs would do the work of closing the bug.  In short, "git
> push" saves you the work of writing a mail because there's a hook for
> it.

Perhaps both approach could be combined.  I still see value in a general
scheme to automate closing applied series that linger on in Debbugs.

[0]  https://lists.gnu.org/archive/html/guix-devel/2023-09/msg00138.html

Change-Ids would also add the benefit that any commit found in Git could
easily be traced back to their submission on the guix-patches or guix
trackers or vice-versa (git log --grep='Change-Id='), as noted by
Giovanni.

The process could go like this:

1. commits of a series pushed to master
2. Savannah sends datagram to a remote machine to trigger the
post-commit job, with the newly pushed commits 'Change-Id' values (a
list of them).
3. The remote machine runs something like 'mumi close-issues [change-id-1
change-id-2 ...]'

In case it couldn't close an issue, it could send a notification to the
submitter: "hey, I've seen some commits of series  landing to
master, but not all of the commits appears to have been pushed, please
check"

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

This is a bit more complex (UDP datagram, mumi database) but it does
useful work for us committers (instead of simply changing the way we
currently do the work).

When not provided any change-id argument, 'mumi close-issues' could run
the process on its complete list of issues.

Since it'd be transparent and requires nothing from a committer, it'd
provide value without having to document yet more processes.

-- 
Thanks,
Maxim



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

2023-09-11 Thread Liliana Marie Prikler
Am Montag, dem 11.09.2023 um 14:36 -0400 schrieb Maxim Cournoyer:
> Hi,
> 
> Liliana Marie Prikler  writes:
> 
> [...]
> 
> > Maybe make that bug-guix so that it matches with the mailing list
> > name,
> > though we also need a wording for when a patch is not a bug, e.g. a
> > simple package upgrade.
> > 
> > WDYT about the following
> >   Applies: [patch] 
> >   Closes: [patch] 
> >   Resolves: [patch] 
> >   Done: [patch] 
> 
> I don't follow; why do we need 'Applies' ?  Why do we need a
> 'namespace'.  Are these things the user would need to manually know
> and enter themselves in their commit messages?
I'm just asking which wording you prefer.  For the tracker, they'd mean
the same as "Fixes", but fixes imho sounds like a bug, which "Update
Emacs to 29.2" isn't.  Thus, something with a more neutral meaning like
"Resolves" might apply better here.

> If so, that's adding rather than reducing friction, and I'm not sure
> it'd gain much traction.  The way I see it, it needs to happen
> automatically.
I mean, the way I imagine is that you type this as part of your message
and then debbugs would do the work of closing the bug.  In short, "git
push" saves you the work of writing a mail because there's a hook for
it.

As for the namespace: you would have to type it on your own – hence why
I prefer the URL approach as that can more easily be copied.  I think
we had a discussion that we don't want to involuntarily trigger stuff
elsewhere, hence why we're marking our own bugs as our own.

Cheers



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

2023-09-11 Thread Maxim Cournoyer
Hi,

Liliana Marie Prikler  writes:

[...]

> Maybe make that bug-guix so that it matches with the mailing list name,
> though we also need a wording for when a patch is not a bug, e.g. a
> simple package upgrade.
>
> WDYT about the following
>   Applies: [patch] 
>   Closes: [patch] 
>   Resolves: [patch] 
>   Done: [patch] 

I don't follow; why do we need 'Applies' ?  Why do we need a
'namespace'.  Are these things the user would need to manually know and
enter themselves in their commit messages?

If so, that's adding rather than reducing friction, and I'm not sure
it'd gain much traction.  The way I see it, it needs to happen
automatically.

-- 
Thanks,
Maxim



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

2023-09-11 Thread Liliana Marie Prikler
Am Montag, dem 11.09.2023 um 10:09 +0200 schrieb Giovanni Biscuolo:
> Hi!
> 
> Liliana Marie Prikler  writes:
> 
> > Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant
> > Cascadian:
> > > I am much more comfortable with the "Fixes" convention of:
> > > 
> > >   Fixes: https://issues.guix.gnu.org/NNN
> 
> OK, I understand Vagrant's concerns: we need a _namespaced_ URI, but
> there is no need that URI is the URL of **one** of our current web
> interfaces, why not the other one? ;-)
Well, I like cool URLs, but maybe that's a nice benefit here.

> 
> I propose:
> 
>  Fixes: [optional bug description] 
> 
> where namespace is the package name, in our case "guix"; fo example:
> 
>  Fixes: Emacs hangs when I press a key 
Maybe make that bug-guix so that it matches with the mailing list name,
though we also need a wording for when a patch is not a bug, e.g. a
simple package upgrade.

WDYT about the following
  Applies: [patch] 
  Closes: [patch] 
  Resolves: [patch] 
  Done: [patch] 



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

2023-09-11 Thread Giovanni Biscuolo
Hi Maxim,

Maxim Cournoyer  writes:

[...]

>> If there is enough consensus I volunteer to collect ideas and send a
>> feature request to the mumi and/or Debbugs devels (if we need Debbugs
>> patches I guess it will be a long term goal)
>
> I don't think any changes to Debbugs would be necessary.  Mumi is
> already able to parse mail headers -- parsing a git trailer should be
> just as simple.

You are right, I'll try to file some feature request for mumi.

Thanks, Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-11 Thread Maxim Cournoyer
Hi,

Simon Tournier  writes:

> Hi Maxim,
>
> Thanks for the explanations.
>
> On Mon, 11 Sept 2023 at 15:47, Maxim Cournoyer
>  wrote:
>
>> >  2. How is Change-Id linked to #65280?
>>
>> Each patch submission ("issue") can have one or multiple commits.  We'd
>> know for sure the series was merged (and thus can be closed) when the
>> set of 'Change-Id's its commits contains have all appeared in the master
>> branch.  The mapping of Debbugs ID <-> set(change-ids) would need to be
>> established by an external tool such as Mumi (which I think is in a good
>> position to do so).
>
> I think it is not straightforward to maintain such mapping.  Because
> Mumi needs to implement a way to extract patches; and there is many
> corner-cases.

It wouldn't need to cover *all* cover cases to be useful; even if it
only worked for 'git send-email' series, it'd still be useful.

> For instance, I am already using a pre-commit hook, how would it work
> in this case?

I didn't think about this case, but if it's common, we could have our
own default pre-commit hook include an escape hatch (e.g., run all
scripts under ~/.config/git/hooks/pre-commit.d) and document that, so
that users could still plug their very own pre-commit hooks.

-- 
Thanks,
Maxim



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

2023-09-11 Thread Simon Tournier
Hi Maxim,

Thanks for the explanations.

On Mon, 11 Sept 2023 at 15:47, Maxim Cournoyer
 wrote:

> >  2. How is Change-Id linked to #65280?
>
> Each patch submission ("issue") can have one or multiple commits.  We'd
> know for sure the series was merged (and thus can be closed) when the
> set of 'Change-Id's its commits contains have all appeared in the master
> branch.  The mapping of Debbugs ID <-> set(change-ids) would need to be
> established by an external tool such as Mumi (which I think is in a good
> position to do so).

I think it is not straightforward to maintain such mapping.  Because
Mumi needs to implement a way to extract patches; and there is many
corner-cases.

For instance, I am already using a pre-commit hook, how would it work
in this case?

Cheers,
simon



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

2023-09-11 Thread Maxim Cournoyer
Hi Giovanni,

Giovanni Biscuolo  writes:

> Hi Maxim,
>
> Maxim Cournoyer  writes:
>
> [...]
>
>>> c. how do we get the issue number of a patch containing "Change-Id"? [1]
>>
>> We'd have to search through the currently opened patches issues; I
>> assume using a tool like the 'mumi' command we already have could do
>> that.
>
> It would be fantastic if we find a way for mumi to index (via xapian)
> the "Change-Id", enabling us to provide a query like this: (is:open and
> change-id:).  I don'r know if this is doable by mumi alone or if it
> needs Debbugs to be able to manage the new "Change-Id" attribute.
>
> If there is enough consensus I volunteer to collect ideas and send a
> feature request to the mumi and/or Debbugs devels (if we need Debbugs
> patches I guess it will be a long term goal)

I don't think any changes to Debbugs would be necessary.  Mumi is
already able to parse mail headers -- parsing a git trailer should be
just as simple.

-- 
Thanks,
Maxim



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

2023-09-11 Thread Maxim Cournoyer
Hi Giovanni,

Giovanni Biscuolo  writes:

> Hi!
>
> Liliana Marie Prikler  writes:
>
>> Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant Cascadian:
>>> I am much more comfortable with the "Fixes" convention of:
>>> 
>>>   Fixes: https://issues.guix.gnu.org/NNN
>
> OK, I understand Vagrant's concerns: we need a _namespaced_ URI, but
> there is no need that URI is the URL of **one** of our current web
> interfaces, why not the other one? ;-)

> IMO this is an implementation detail we can easily fix once we find a
> consensus on introducing this requirement in the Guix guidelines on
> committing.

Agreed.  I think a simple git trailer such as:

--8<---cut here---start->8---
Fixes: bug#65738 (java-ts-mode tests)
--8<---cut here---end--->8---

Would be nice, where bug#N is already known and shown as a URL in
Emacs when the bug-reference minor mode is enabled (it's the case
working with Magit), so bugs can be consulted easily without any context
switching.

-- 
Thanks,
Maxim



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

2023-09-11 Thread Maxim Cournoyer
Hi Simon,

Simon Tournier  writes:

> Hi Maxim,
>
> On Sat, 09 Sep 2023 at 19:50, Maxim Cournoyer  
> wrote:
>
 --8<---cut here---start->8---
 random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
   git hash-object --stdin)
 --8<---cut here---end--->8---
>>>
>>> That seems like it would only work if the patch was identical, as
>>> opposed to a slightly rebased patch on top of newer patches on master?
>>>
>>> How can you correlate Change-Id to a patch in the tracker?
>>
>> The Change-Id stays the same unless you manually edit it out of your
>> commit message when amending / rebasing, so the commit hash may change
>> while the Change-Id stays the same.  So you can rebase your feature
>> branch on master and share a v2, whose existing commits will have the
>> same Change-Ids (newly added commits would get their own Change-Id
>> trailer).
>
> I am sorry if I am slow but I do not understand.
>
> $ git var GIT_COMMITTER_IDENT
> Simon Tournier  1694428674 +0200
> $ git var GIT_COMMITTER_IDENT
> Simon Tournier  1694428800 +0200
>
> Therefore this Change-Id can be different for the same series, depending
> when I am locally committing.  No?
>
> And sorry if I am slow but I am also missing your answer about “How can
> you correlate Change-Id to a patch in the tracker?”.  How is this
> Change-Id correlated to the Debbugs number?

The commit hook job is simple: generate a unique ID.  The magic is that
it only adds it *once* to a commit message; if it finds the 'Change-Id:'
line already present in it, it won't refresh its value or add a new one.
So it's immutable once added, preserved in the Git commit message
itself.

> Let take an example. :-) Assume Change-Id was used for your submission
> bug#65280 about Qt.  It reads many patches and we have:
>
> 02/59
> 1717c8a233b7fda3a10aabc061168c71317f883e
> AuthorDate: Fri Aug 11 15:26:14 2023 -0400
>
> 59/59
> 0a77b869322274030c4c0e8315ddea802da44c92
> AuthorDate: Tue Aug 15 16:20:10 2023 -0400
>
> From my understanding,
>
>  1. GIT_COMMITTER_IDENT depends on time so these two commits would have
> a different Change-Id, no?

Correct.  The commit hook adds a unique ID *per* commit.

>  2. How is Change-Id linked to #65280?

Each patch submission ("issue") can have one or multiple commits.  We'd
know for sure the series was merged (and thus can be closed) when the
set of 'Change-Id's its commits contains have all appeared in the master
branch.  The mapping of Debbugs ID <-> set(change-ids) would need to be
established by an external tool such as Mumi (which I think is in a good
position to do so).

I hope that helps!

-- 
Thanks,
Maxim



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

2023-09-11 Thread Simon Tournier
Hi Maxim,

On Sat, 09 Sep 2023 at 19:50, Maxim Cournoyer  wrote:

>>> --8<---cut here---start->8---
>>> random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
>>>   git hash-object --stdin)
>>> --8<---cut here---end--->8---
>>
>> That seems like it would only work if the patch was identical, as
>> opposed to a slightly rebased patch on top of newer patches on master?
>>
>> How can you correlate Change-Id to a patch in the tracker?
>
> The Change-Id stays the same unless you manually edit it out of your
> commit message when amending / rebasing, so the commit hash may change
> while the Change-Id stays the same.  So you can rebase your feature
> branch on master and share a v2, whose existing commits will have the
> same Change-Ids (newly added commits would get their own Change-Id
> trailer).

I am sorry if I am slow but I do not understand.

--8<---cut here---start->8---
$ git var GIT_COMMITTER_IDENT
Simon Tournier  1694428674 +0200
$ git var GIT_COMMITTER_IDENT
Simon Tournier  1694428800 +0200
--8<---cut here---end--->8---

Therefore this Change-Id can be different for the same series, depending
when I am locally committing.  No?

And sorry if I am slow but I am also missing your answer about “How can
you correlate Change-Id to a patch in the tracker?”.  How is this
Change-Id correlated to the Debbugs number?


Let take an example. :-) Assume Change-Id was used for your submission
bug#65280 about Qt.  It reads many patches and we have:

02/59
1717c8a233b7fda3a10aabc061168c71317f883e
AuthorDate: Fri Aug 11 15:26:14 2023 -0400

59/59
0a77b869322274030c4c0e8315ddea802da44c92
AuthorDate: Tue Aug 15 16:20:10 2023 -0400

>From my understanding,

 1. GIT_COMMITTER_IDENT depends on time so these two commits would have
a different Change-Id, no?

 2. How is Change-Id linked to #65280?

Cheers,
simon





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

2023-09-11 Thread Giovanni Biscuolo
Hi!

Liliana Marie Prikler  writes:

> Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant Cascadian:
>> I am much more comfortable with the "Fixes" convention of:
>> 
>>   Fixes: https://issues.guix.gnu.org/NNN

OK, I understand Vagrant's concerns: we need a _namespaced_ URI, but
there is no need that URI is the URL of **one** of our current web
interfaces, why not the other one? ;-)

IMO this is an implementation detail we can easily fix once we find a
consensus on introducing this requirement in the Guix guidelines on
committing.

> I like the idea, but we should also consider the bugs.gnu.org address
> here as well as the convention of putting it into angular brackets.  In
> fact, I might even prefer it if the convention was
>   Fixes: Bug description 
> where bug description is a (possibly empty) name for the bug such as
> "Emacs hangs when I press a key" or something.

I agree that an (optional) bug description might be useful (could also
be automatically added by some cool etc/committer.scm funcion?)

I propose:

 Fixes: [optional bug description] 

where namespace is the package name, in our case "guix"; fo example:

 Fixes: Emacs hangs when I press a key 

WDYT?

> As for when to send it, remember that we already send a bunch of mails
> to guix-comm...@gnu.org as our commit hook?  I think it shouldn't be
> too hard to search for the fixes line and send it to debbugs control.

Do you please know whare to get that scripts, just to have a quick look
and understand how we could eventually add a function for automatic bug
closing?

Thanks! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-11 Thread Giovanni Biscuolo
Hi Maxim,

Maxim Cournoyer  writes:

[...]

>> c. how do we get the issue number of a patch containing "Change-Id"? [1]
>
> We'd have to search through the currently opened patches issues; I
> assume using a tool like the 'mumi' command we already have could do
> that.

It would be fantastic if we find a way for mumi to index (via xapian)
the "Change-Id", enabling us to provide a query like this: (is:open and
change-id:).  I don'r know if this is doable by mumi alone or if it
needs Debbugs to be able to manage the new "Change-Id" attribute.

If there is enough consensus I volunteer to collect ideas and send a
feature request to the mumi and/or Debbugs devels (if we need Debbugs
patches I guess it will be a long term goal)

>> [1] right now how do we get the issue number of a committed patch?
>
> There's no direct mapping.  You have to search by subject name.

IMO a link like this is _very_ useful in helping bug tracking management
(and also for historians :-) ) and we should find a way to manage that.

Also for this reason (other that possibly automated bug closing) IMO
committers should add a "Fixes: #" signature (or what
is called in Git) to each commit with an associeted bug report.

WDYT?

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-09 Thread Liliana Marie Prikler
Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant Cascadian:
> I am much more comfortable with the "Fixes" convention of:
> 
>   Fixes: https://issues.guix.gnu.org/NNN
I like the idea, but we should also consider the bugs.gnu.org address
here as well as the convention of putting it into angular brackets.  In
fact, I might even prefer it if the convention was
  Fixes: Bug description 
where bug description is a (possibly empty) name for the bug such as
"Emacs hangs when I press a key" or something.


As for when to send it, remember that we already send a bunch of mails
to guix-comm...@gnu.org as our commit hook?  I think it shouldn't be
too hard to search for the fixes line and send it to debbugs control.

WDYT?



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

2023-09-09 Thread Maxim Cournoyer
Hi Vagrant,

Vagrant Cascadian  writes:

[...]

>> Did you see my message about integrating a commit-hook similar to what
>> Gerrit uses?  It produces unique ID such as:
>>
>> --8<---cut here---start->8---
>> Change-Id: I9b86781869d80eda347659f0c009b8dfe09bdfd0
>> --8<---cut here---end--->8---
>>
>> at the bottom (it's a git trailer) of a commit message.  Being unique,
>> these could be used to match if a patch on the tracker has been merged
>> in the master branch.
>>
>> Attached is what the hook looks like.  The random 'Change-Id' is
>> generated via
>>
>> --8<---cut here---start->8---
>> random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
>>   git hash-object --stdin)
>> --8<---cut here---end--->8---
>
> That seems like it would only work if the patch was identical, as
> opposed to a slightly rebased patch on top of newer patches on master?
>
> How can you correlate Change-Id to a patch in the tracker?

The Change-Id stays the same unless you manually edit it out of your
commit message when amending / rebasing, so the commit hash may change
while the Change-Id stays the same.  So you can rebase your feature
branch on master and share a v2, whose existing commits will have the
same Change-Ids (newly added commits would get their own Change-Id
trailer).

> Would it break git commit signatures?

It doesn't impact git commit signatures.

-- 
Thanks,
Maxim



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

2023-09-09 Thread Maxim Cournoyer
Hi Simon,

Simon Tournier  writes:

> Hi,
>
> On Wed, 06 Sep 2023 at 22:01, Maxim Cournoyer  
> wrote:
>
>> We could use Gerrit's commit hook that adds a unique ID as a git
>> trailer.  Then it should become possible to
>
> Do we still already have Gerrit running?

I was not suggesting to host Gerrit, simply to borrow its git commit
hook, which inserts a unique ID in the commit message (as a git
trailer), which can then be used to keep track of a commit even when
it's rebased or amended.

-- 
Thanks,
Maxim



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

2023-09-09 Thread Maxim Cournoyer
Hi Giovanni,

Giovanni Biscuolo  writes:

[...]

>> We could use Gerrit's commit hook that adds a unique ID as a git
>> trailer.
>
> Do you mean "commit-msg" hook as documented here:
> https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
> ?
>
>
> The Gerrit Code Review supplied implementation of this hook is a short shell 
> script which automatically inserts a globally unique Change-Id tag in the 
> footer of a commit message. When present, Gerrit uses this tag to track 
> commits across cherry-picks and rebases.
>
>> Then it should become possible to
>>
>> 1. Check if all items of a series have appeared in the git history
>> 2. If so, close the associated issue if it was still open
>
> Thinking out loud:
>
> a. each contributed patch will have a unique Change-Id, persistent
> across rebases (and git commit --amend), and every new patch version
> (produced during patch revision) will have the same Change-Id; this is
> valid for all commits in a patch set

Correct.

> b. when all "Change-Id"s of patches contained in a patch set are listed
> in the git history (of one of the official branches) the associated
> issue can be closed

That was my idea.

> c. how do we get the issue number of a patch containing "Change-Id"? [1]

We'd have to search through the currently opened patches issues; I
assume using a tool like the 'mumi' command we already have could do
that.

> [1] right now how do we get the issue number of a committed patch?

There's no direct mapping.  You have to search by subject name.

-- 
Thanks,
Maxim



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

2023-09-07 Thread Development of GNU Guix and the GNU System distribution.
Hi Vagrant!

On Thu, Sep 7, 2023 at 9:13 AM Vagrant Cascadian  wrote:
>
> So... 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.

Good catch! Surely, many downstream maintainers face that issue.

It's been a while since I hacked on Salsa. Regardless of the outcome
here, can you disable the standard webhook and add your own?

Kind regards
Felix



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

2023-09-07 Thread Vagrant Cascadian
On 2023-09-07, Giovanni Biscuolo wrote:
> Hi Maxim and Ludovic,
> Maxim Cournoyer  writes:
>> Giovanni Biscuolo  writes:
..
>>> When I asket I though the best way would be to scan for a string like
>>> "Close #" in the commit message (the committer should add
>>> such a string) but probably this can be avoided: the bug can be closed
>>> when a patch is committed to one of the listed official brances (master,
>>> core-updates, etc.)
>>
>> You mean by simply matching the subject?  Maybe that could work.
>
> Thinking twice maybe it's better to add a "Close: #" to the commit
> message.

So... 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.

Currently that is:

  
https://salsa.debian.org/salsa/salsa-webhook/-/blob/997412ad4b1d89fd3227d5409471bdf71abcf863/helpers/bugs.rb#L19

  SCAN_BUGS = 
/Closes:\s+(?:Bug)?#(?:(\d{4,8})\b)(?:,?\s*(?:Bug)?#(?:(\d{4,8})\b))*/i

"Close: #" is a bit nervously close; I could see a stray typo of an
extra "s" triggering the occasional very annoying problem.

I am much more comfortable with the "Fixes" convention of:

  Fixes: https://issues.guix.gnu.org/NNN


So, while I cannot insist here... I can plead a bit... and a namespaced
URL is kind of better in some ways anyways. :)


live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-07 Thread Vagrant Cascadian
On 2023-09-07, Maxim Cournoyer wrote:
> Felix Lechner  writes:
>> On Thu, Sep 7, 2023 at 4:08 AM Giovanni Biscuolo  wrote:
>>>
>>> close the bugs that are listed in
>>> the commit message
...
> Did you see my message about integrating a commit-hook similar to what
> Gerrit uses?  It produces unique ID such as:
>
> --8<---cut here---start->8---
> Change-Id: I9b86781869d80eda347659f0c009b8dfe09bdfd0
> --8<---cut here---end--->8---
>
> at the bottom (it's a git trailer) of a commit message.  Being unique,
> these could be used to match if a patch on the tracker has been merged
> in the master branch.
>
> Attached is what the hook looks like.  The random 'Change-Id' is
> generated via
>
> --8<---cut here---start->8---
> random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
>   git hash-object --stdin)
> --8<---cut here---end--->8---

That seems like it would only work if the patch was identical, as
opposed to a slightly rebased patch on top of newer patches on master?

How can you correlate Change-Id to a patch in the tracker?

Would it break git commit signatures?


live well,
  vagrant


signature.asc
Description: PGP signature


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

2023-09-07 Thread Simon Tournier
Hi,

On Wed, 06 Sep 2023 at 22:01, Maxim Cournoyer  wrote:

> We could use Gerrit's commit hook that adds a unique ID as a git
> trailer.  Then it should become possible to

Do we still already have Gerrit running?

Cheers,
simon



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

2023-09-07 Thread Giovanni Biscuolo
Hi,

Giovanni Biscuolo  writes:

[...]

>>> The first thing we need is a server side git post-receive hook on
>>> Savannah, I've opened the sr#110928 support request:
>>> https://savannah.nongnu.org/support/index.php?110928
>>
>> It's something that the Savannah folks would need to maintain
>> themselves, right?
>
> Forgot to mention that I'm pretty sure a post-receive server side hook
> is already running (and maintained) for our guix.git repo on Savannah,
> it's the one that sends notifications to guix-commits mailing list
> https://lists.gnu.org/mailman/listinfo/guix-commits

Regarding server side git hooks, I forgot to mention that on 2023-08-31
a new commit-hook is available on Savannah (installation must be
requested per-project):

git post-receive UDP syndication
https://savannah.gnu.org/news/?id=10508

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

A new commit-hook is available to install for git repositories that will send a 
single Datagram via UDP after each successful commit.  This can be useful for 
continuous integration (CI) schemes and elsewise when a push driven model is 
prefered to (e.g) regularly repolling upstream when changes may or may not have 
occured. 

To request installation please open a ticket with the Savannah Administration 
project:

[...]

The (sh, GPLv3+) post-receive script source, detail on how the Datagram is 
structured, and example "receiver" scripts (in perl) can be found here:

  https://git.sr.ht/~mplscorwin/git-udp-syndicate

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

Maybe this hook is useful for comminication with the QA service(s).

Maybe this hook could be adapted to close bugs instead of send an UDP
datagram.

Happy hacking! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-07 Thread Giovanni Biscuolo
Hi Felix,

Felix Lechner  writes:

> Hi Gio',
>
> On Thu, Sep 7, 2023 at 4:08 AM Giovanni Biscuolo  wrote:
>>
>> close the bugs that are listed in
>> the commit message
>
> Perhaps you'd like to see Debian's hook [1] for the Salsa web forge
> (which is controversially based on Gitlab).

[...]

Thank you for the reference, it'll be an useful example.

A ruby interpreter dependency in this case probably is too much, but we
can always rewrite that :-)

Happy hacking! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-07 Thread Giovanni Biscuolo
Hi Maxim,

Maxim Cournoyer  writes:

> Simon Tournier  writes:

[...]

>> Maybe patchwork already running (I think) could help, trying to
>> regularly rebase the branch dedicated to the submission on the top of
>> master, then if all is fine, somehow the two heads from the master
>> branch and the dedicated branch should match, and it would indicate the
>> patches are included and it is safe to close.  More or less. :-)
>
> We could use Gerrit's commit hook that adds a unique ID as a git
> trailer.

Do you mean "commit-msg" hook as documented here:
https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
?

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

The Gerrit Code Review supplied implementation of this hook is a short shell 
script which automatically inserts a globally unique Change-Id tag in the 
footer of a commit message. When present, Gerrit uses this tag to track commits 
across cherry-picks and rebases.

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

> Then it should become possible to
>
> 1. Check if all items of a series have appeared in the git history
> 2. If so, close the associated issue if it was still open

Thinking out loud:

a. each contributed patch will have a unique Change-Id, persistent
across rebases (and git commit --amend), and every new patch version
(produced during patch revision) will have the same Change-Id; this is
valid for all commits in a patch set

b. when all "Change-Id"s of patches contained in a patch set are listed
in the git history (of one of the official branches) the associated
issue can be closed

c. how do we get the issue number of a patch containing "Change-Id"? [1]

Did I miss something?

Thanks, Gio'


[1] right now how do we get the issue number of a committed patch?

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-07 Thread Maxim Cournoyer
Hi,

Felix Lechner  writes:

> Hi Gio',
>
> On Thu, Sep 7, 2023 at 4:08 AM Giovanni Biscuolo  wrote:
>>
>> close the bugs that are listed in
>> the commit message
>
> Perhaps you'd like to see Debian's hook [1] for the Salsa web forge
> (which is controversially based on Gitlab).
>
> The regex does not work properly for more than two bugs, however, so
> here is my proposed fix. [2] The merge request was closed
> automatically when I began contributing to Guix.

Did you see my message about integrating a commit-hook similar to what
Gerrit uses?  It produces unique ID such as:

--8<---cut here---start->8---
Change-Id: I9b86781869d80eda347659f0c009b8dfe09bdfd0
--8<---cut here---end--->8---

at the bottom (it's a git trailer) of a commit message.  Being unique,
these could be used to match if a patch on the tracker has been merged
in the master branch.

Attached is what the hook looks like.  The random 'Change-Id' is
generated via

--8<---cut here---start->8---
random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
  git hash-object --stdin)
--8<---cut here---end--->8---



commit-msg
Description: Binary data

-- 
Thanks,
Maxim


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

2023-09-07 Thread Development of GNU Guix and the GNU System distribution.
Hi Gio',

On Thu, Sep 7, 2023 at 4:08 AM Giovanni Biscuolo  wrote:
>
> close the bugs that are listed in
> the commit message

Perhaps you'd like to see Debian's hook [1] for the Salsa web forge
(which is controversially based on Gitlab).

The regex does not work properly for more than two bugs, however, so
here is my proposed fix. [2] The merge request was closed
automatically when I began contributing to Guix.

Kind regards
Felix

[1] 
https://salsa.debian.org/salsa/salsa-webhook/-/blob/997412ad4b1d89fd3227d5409471bdf71abcf863/helpers/bugs.rb#L19
[2] https://salsa.debian.org/salsa/salsa-webhook/-/merge_requests/22



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

2023-09-07 Thread Giovanni Biscuolo
Hi Simon,

Simon Tournier  writes:

> On Wed, 06 Sep 2023 at 12:14, Maxim Cournoyer  
> wrote:
>
>>> Let's avoid manual gardening as much as possible! :-)
>>
>> I like the idea!
>
> I think that automatizing is not trivial.  Sadly.

If we "restrict" the automation to "close the bugs that are listed in
the commit message" do you think it's doable?

[...]

> The potential issue is the number of false-positive;

In the context given above, the only way to have a false positive is
that the committer give a wrong bug number, right?

> closing and the submission is not applied.

I don't understand: what do you mean by "submission"?

By design:

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

The post-receive hook runs after the entire process is completed and can be 
used to update other services or notify users.

--8<---cut here---end--->8---
(https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks)

In this case the "other service update" is "close bug " and is
guaranteed to be done after the commit is applied.

> Maybe patchwork already running (I think) could help, trying to
> regularly rebase the branch dedicated to the submission on the top of
> master, then if all is fine, somehow the two heads from the master
> branch and the dedicated branch should match, and it would indicate the
> patches are included and it is safe to close.  More or less. :-)

I'm lost :-D

> That’s said, I always find annoying to loose the track between the Git
> history and the discussion that happened in the tracker.  Sometimes,
> rational of some details of the implementation had been discussed in the
> tracker and it is impossible to find then back.  Therefore, I would be
> in favor to add ’Close #1234’ in the commit message, say the first one
> from the series tracked by #1234.  Doing so, it would ease automatic
> management of guix-patches.  However, it would add again some burden on
> committer shoulder.

I completely agree that (often, seldom?) we miss traceability if we do
not link a commit with a bug report (when present) and vice versa (when
a bug report is resolved by a series of commit)

> Similarly, we are already adding in the commit message something like
> ’Fixes ’.

Is this an informal convention or is this documented somewhere?

> And that could be used for closing.

Yes, we can use al list of keywords for closing (Closes, Close, Fix,
Fixes, etc.), but for the bug report I'd use only a number, the URL
really does not matter

> Again, the concern is about false-positive; closing when it should not
> be.

Modulo a programming error in the script, the only way would be to write
the wrong bug number after the "keyword" and IMO this is similar write
the wrong bug number in the "To:" field when closing a bug via email.

> Well, I think that automatizing is not trivial. :-)

Not trivial but automatable, IMO :-D

Thanks! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-07 Thread Giovanni Biscuolo
Hi Maxim and Ludovic,

I'm including you, Ludovic, becaus in the past you requested a similar
git hook installation for Guix and maybe you have more info

Maxim Cournoyer  writes:

> Giovanni Biscuolo  writes:

[...]

OK, I really found the wrong examples as Christopher pointed out, but my
proposal still holds valid :-D

>> IMO we need a way automatically close this kind of bug reports... or am
>> I missing something?
>>
>> Let's avoid manual gardening as much as possible! :-)
>
> I like the idea!

Fine, let's try to do it!

>> The first thing we need is a server side git post-receive hook on
>> Savannah, I've opened the sr#110928 support request:
>> https://savannah.nongnu.org/support/index.php?110928
>
> It's something that the Savannah folks would need to maintain
> themselves, right?

Forgot to mention that I'm pretty sure a post-receive server side hook
is already running (and maintained) for our guix.git repo on Savannah,
it's the one that sends notifications to guix-commits mailing list
https://lists.gnu.org/mailman/listinfo/guix-commits

Maybe that server side hook is common to all Savannah hosted projects,
but in this case it sould be configurable per-project since the "From:"
and "To:" email headers must be project related.

...or that post-receive hook has been installed the Savannah admin of
the Guix project?

I don't know what is the policy for "custom" server side web hooks: for
sure Someone™ with ssh access to /srv/git/guix.git/hooks/ should
/install/ such a script.

I've not found any reference to "per project" git hooks in the Savannah
documentation
(https://duckduckgo.com/?q=git+hook+site%3Ahttps%3A%2F%2Fsavannah.gnu.org%2Fmaintenance%2FFrontPage%2F=web)

I have found many support request mentioning git hooks:
https://savannah.gnu.org/search/?words1=git+hook_of_search=support=Search=1#options

In particular, this support threads:

- sr #110614: auctex-diffs emails should be in the same format as emacs-diffs
https://savannah.nongnu.org/support/?func=detailitem_id=110614

- sr #110594: git push returns "hints" about ignored hooks
https://savannah.nongnu.org/support/?func=detailitem_id=110594

- sr #110482: Install irker hooks in poke.git
https://savannah.nongnu.org/support/?func=detailitem_id=110482

- sr #109104: Add Git 'update' hook for Guix repositories
https://savannah.nongnu.org/support/?func=detailitem_id=109104

Makes me believe that server-side hooks are something that must be
installed per-project by Savannah sysadmins

>> When I asket I though the best way would be to scan for a string like
>> "Close #" in the commit message (the committer should add
>> such a string) but probably this can be avoided: the bug can be closed
>> when a patch is committed to one of the listed official brances (master,
>> core-updates, etc.)
>
> You mean by simply matching the subject?  Maybe that could work.

Thinking twice maybe it's better to add a "Close: #" to the commit
message.

Now... let's find (or develop) such a post-receive hook!

Happy hacking! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature


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

2023-09-06 Thread Maxim Cournoyer
Hi,

Simon Tournier  writes:

> Hi,
>
> On Wed, 06 Sep 2023 at 12:14, Maxim Cournoyer  
> wrote:
>
>>> Let's avoid manual gardening as much as possible! :-)
>>
>> I like the idea!
>
> I think that automatizing is not trivial.  Sadly.
>
> There are many corner cases:
>
>  1. series as attachement
>  2. not all the series is applied
>  3. commit message had been tweaked
>  4. etc.
>
> The potential issue is the number of false-positive; closing and the
> submission is not applied.
>
> Maybe patchwork already running (I think) could help, trying to
> regularly rebase the branch dedicated to the submission on the top of
> master, then if all is fine, somehow the two heads from the master
> branch and the dedicated branch should match, and it would indicate the
> patches are included and it is safe to close.  More or less. :-)

We could use Gerrit's commit hook that adds a unique ID as a git
trailer.  Then it should become possible to

1. Check if all items of a series have appeared in the git history
2. If so, close the associated issue if it was still open

-- 
Thanks,
Maxim



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

2023-09-06 Thread Simon Tournier
Hi,

On Wed, 06 Sep 2023 at 12:14, Maxim Cournoyer  wrote:

>> Let's avoid manual gardening as much as possible! :-)
>
> I like the idea!

I think that automatizing is not trivial.  Sadly.

There are many corner cases:

 1. series as attachement
 2. not all the series is applied
 3. commit message had been tweaked
 4. etc.

The potential issue is the number of false-positive; closing and the
submission is not applied.

Maybe patchwork already running (I think) could help, trying to
regularly rebase the branch dedicated to the submission on the top of
master, then if all is fine, somehow the two heads from the master
branch and the dedicated branch should match, and it would indicate the
patches are included and it is safe to close.  More or less. :-)

That’s said, I always find annoying to loose the track between the Git
history and the discussion that happened in the tracker.  Sometimes,
rational of some details of the implementation had been discussed in the
tracker and it is impossible to find then back.  Therefore, I would be
in favor to add ’Close #1234’ in the commit message, say the first one
from the series tracked by #1234.  Doing so, it would ease automatic
management of guix-patches.  However, it would add again some burden on
committer shoulder.

Similarly, we are already adding in the commit message something like
’Fixes ’.  And that could be
used for closing.  Again, the concern is about false-positive; closing
when it should not be.

Well, I think that automatizing is not trivial. :-)


Cheers,
simon



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

2023-09-06 Thread Maxim Cournoyer
Hi Giovanni,

Giovanni Biscuolo  writes:

> Hello,
>
> often bug reports related to patches are left open even after the
> patch/patchset have been applied, the last example is a batch of Debbugs
> manual gardening from Vagrant last Fri and Sat when he closed more than
> 20 bugs with messages similar to this one:

[...]

> IMO we need a way automatically close this kind of bug reports... or am
> I missing something?
>
> Let's avoid manual gardening as much as possible! :-)

I like the idea!

> The first thing we need is a server side git post-receive hook on
> Savannah, I've opened the sr#110928 support request:
> https://savannah.nongnu.org/support/index.php?110928

It's something that the Savannah folks would need to maintain
themselves, right?

> When I asket I though the best way would be to scan for a string like
> "Close #" in the commit message (the committer should add
> such a string) but probably this can be avoided: the bug can be closed
> when a patch is committed to one of the listed official brances (master,
> core-updates, etc.)

You mean by simply matching the subject?  Maybe that could work.

-- 
Thanks,
Maxim



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

2023-09-06 Thread Christopher Baines

Giovanni Biscuolo  writes:

> Hello,
>
> often bug reports related to patches are left open even after the
> patch/patchset have been applied, the last example is a batch of Debbugs
> manual gardening from Vagrant last Fri and Sat when he closed more than
> 20 bugs with messages similar to this one:
>
>
>  rofi-wayland was added in:
>
>  04b5450ad852735dfa50961d3afc789b2e52b407 gnu: Add rofi-wayland.
>
>  And updated to a newer version in:
>
>  19c042ddf80533ba7a615b424dedf9647ca65b0f gnu: rofi-wayland: Update to 
> 1.7.5+wayland2.
>
>  Marking as done.
>
> (https://yhetil.org/guix/87zg25r0id.fsf@wireframe/)
>
> IMO we need a way automatically close this kind of bug reports... or am
> I missing something?

I think the example you give doesn't relate to what you're looking at
below (a post-receive hook).

There were at least two different issues with patches for adding
rofi-wayland [1] and [2].

1: https://issues.guix.gnu.org/53717
2: https://issues.guix.gnu.org/59241

One improvement I can think of here is that QA should highlight that
some of the changes in each of those patch series can be found in
another patch series.

That would then make it easier to both issues to be closed if that's
appropriate.


signature.asc
Description: PGP signature


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

2023-09-06 Thread Giovanni Biscuolo
Hello,

often bug reports related to patches are left open even after the
patch/patchset have been applied, the last example is a batch of Debbugs
manual gardening from Vagrant last Fri and Sat when he closed more than
20 bugs with messages similar to this one:

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

 rofi-wayland was added in:

 04b5450ad852735dfa50961d3afc789b2e52b407 gnu: Add rofi-wayland.

 And updated to a newer version in:

 19c042ddf80533ba7a615b424dedf9647ca65b0f gnu: rofi-wayland: Update to 
1.7.5+wayland2.

 Marking as done.

--8<---cut here---end--->8---
(https://yhetil.org/guix/87zg25r0id.fsf@wireframe/)

IMO we need a way automatically close this kind of bug reports... or am
I missing something?

Let's avoid manual gardening as much as possible! :-)

The first thing we need is a server side git post-receive hook on
Savannah, I've opened the sr#110928 support request:
https://savannah.nongnu.org/support/index.php?110928

When I asket I though the best way would be to scan for a string like
"Close #" in the commit message (the committer should add
such a string) but probably this can be avoided: the bug can be closed
when a patch is committed to one of the listed official brances (master,
core-updates, etc.)

WDYT?

Ciao, Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature