[PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

  • Done
  • quality assurance status badge
Details
4 participants
  • Maxim Cournoyer
  • Mathieu Othacehe
  • Ricardo Wurmus
  • Simon Tournier
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 20 Dec 2022 14:58
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221220135810.28175-1-maxim.cournoyer@gmail.com
Along with the correct '.patman' configuration file for Guix, this makes it
possible to invoke the 'patman' command and have it add all the required
'--cc' directives, as computed by 'etc/teams.scm get-maintainers
<patch-file>'.


Maxim Cournoyer (2):
teams: Add a "get-maintainer" command.
.patman: New configuration file.

.patman | 9 +++++++++
etc/teams.scm.in | 21 ++++++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 .patman


base-commit: f28ca2447c5e2eef1ba6a3a11587380a665b0e26
--
2.38.1
M
M
Maxim Cournoyer wrote on 20 Dec 2022 15:13
[PATCH 1/2] teams: Add a "get-maintainer" command.
(address . 60218@debbugs.gnu.org)
20221220141330.30372-1-maxim.cournoyer@gmail.com
This can be used as a compatibility mode with the get_maintainer.pl Perl
script included in the Linux (or U-Boot) source tree.

* etc/teams.scm.in (git-patch->commit-id): New procedure.
(main) <get-maintainer>: Register new command. Document it.
---

etc/teams.scm.in | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)

Toggle diff (64 lines)
diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index aa38a3b798..4f02df79d5 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -5,6 +5,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2022 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2022 Mathieu Othacehe <othacehe@gnu.org>
+;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -34,6 +35,7 @@
(ice-9 format)
(ice-9 regex)
(ice-9 match)
+ (ice-9 rdelim)
(guix ui)
(git))
@@ -608,6 +610,15 @@ (define (diff-revisions rev-start rev-end)
(const 0))
files))
+(define (git-patch->commit-id file)
+ "Parse the commit ID from the first line of FILE, a patch produced with git."
+ (call-with-input-file file
+ (lambda (port)
+ (let ((m (string-match "^From ([0-9a-f]{40})" (read-line port))))
+ (unless m
+ (error "invalid patch file:" file))
+ (match:substring m 1)))))
+
(define (main . args)
(match args
@@ -616,6 +627,14 @@ (define (main . args)
(("cc-members" rev-start rev-end)
(apply cc (find-team-by-scope
(diff-revisions rev-start rev-end))))
+ (("get-maintainer" patch-file)
+ (let* ((rev-end (git-patch->commit-id patch-file))
+ (rev-start (string-append rev-end "^")))
+ (apply main "list-members"
+ (map symbol->string
+ (map team-id
+ (find-team-by-scope
+ (diff-revisions rev-start rev-end)))))))
(("list-teams" . args)
(list-teams))
(("list-members" . team-names)
@@ -631,6 +650,7 @@ (define (main . args)
cc <team-name> get git send-email flags for cc-ing <team-name>
cc-members <start> <end> cc teams related to files changed between revisions
list-teams list teams and their members
- list-members <team-name> list members belonging to <team-name>~%"))))
+ list-members <team-name> list members belonging to <team-name>~%
+ get-maintainer <patch> compatibility mode with Linux get_maintainer.pl"))))
(apply main (cdr (command-line)))

base-commit: f28ca2447c5e2eef1ba6a3a11587380a665b0e26
--
2.38.1
M
M
Maxim Cournoyer wrote on 20 Dec 2022 15:13
[PATCH 2/2] .patman: New configuration file.
(address . 60218@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221220141330.30372-2-maxim.cournoyer@gmail.com
* .patman: New file.

---

.patman | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 .patman

Toggle diff (17 lines)
diff --git a/.patman b/.patman
new file mode 100644
index 0000000000..4708bb5ea8
--- /dev/null
+++ b/.patman
@@ -0,0 +1,9 @@
+# This config file allows for Patchwork integration with
+# https://patches.guix-patches.cbaines.net/.
+[settings]
+project: guix-patches
+patchwork_url: https://patches.guix-patches.cbaines.net
+add_signoff: False
+# TODO: enable check_patch
+check_patch: False
+get_maintainer_script: etc/teams.scm get-maintainer
--
2.38.1
R
R
Ricardo Wurmus wrote on 25 Dec 2022 00:15
Re: [PATCH 1/2] teams: Add a "get-maintainer" command.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87tu1kbd43.fsf@elephly.net
Hi Maxim,

Toggle quote (6 lines)
> This can be used as a compatibility mode with the get_maintainer.pl Perl
> script included in the Linux (or U-Boot) source tree.
>
> * etc/teams.scm.in (git-patch->commit-id): New procedure.
> (main) <get-maintainer>: Register new command. Document it.

Interesting.

Toggle quote (8 lines)
> @@ -616,6 +627,14 @@ (define (main . args)
> (("cc-members" rev-start rev-end)
> (apply cc (find-team-by-scope
> (diff-revisions rev-start rev-end))))
> + (("get-maintainer" patch-file)
> + (let* ((rev-end (git-patch->commit-id patch-file))
> + (rev-start (string-append rev-end "^")))

This is to get the changes introduced by this patch-file right? In a
format that allows you to use “diff-revisions” below, which you need to
run find-team-by-scope.

Toggle quote (6 lines)
> + (apply main "list-members"
> + (map symbol->string
> + (map team-id
> + (find-team-by-scope
> + (diff-revisions rev-start rev-end)))))))

Here I’d do

(map (compose symbol->string team-id) …)

instead of mapping twice.

I haven’t used get_maintainer.pl before, but I don’t object to this
change if it’s useful to you.

--
Ricardo
M
M
Maxim Cournoyer wrote on 27 Dec 2022 04:19
(name . Ricardo Wurmus)(address . rekado@elephly.net)
87cz858re8.fsf@gmail.com
Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

Toggle quote (22 lines)
> Hi Maxim,
>
>> This can be used as a compatibility mode with the get_maintainer.pl Perl
>> script included in the Linux (or U-Boot) source tree.
>>
>> * etc/teams.scm.in (git-patch->commit-id): New procedure.
>> (main) <get-maintainer>: Register new command. Document it.
>
> Interesting.
>
>> @@ -616,6 +627,14 @@ (define (main . args)
>> (("cc-members" rev-start rev-end)
>> (apply cc (find-team-by-scope
>> (diff-revisions rev-start rev-end))))
>> + (("get-maintainer" patch-file)
>> + (let* ((rev-end (git-patch->commit-id patch-file))
>> + (rev-start (string-append rev-end "^")))
>
> This is to get the changes introduced by this patch-file right? In a
> format that allows you to use “diff-revisions” below, which you need to
> run find-team-by-scope.

Yes! The get-maintainer.pl script expects a single patch file rather
than two git refspecs.

Toggle quote (12 lines)
>> + (apply main "list-members"
>> + (map symbol->string
>> + (map team-id
>> + (find-team-by-scope
>> + (diff-revisions rev-start rev-end)))))))
>
> Here I’d do
>
> (map (compose symbol->string team-id) …)
>
> instead of mapping twice.

Thanks, that's better. Adjusted locally.

Toggle quote (3 lines)
> I haven’t used get_maintainer.pl before, but I don’t object to this
> change if it’s useful to you.

It's useful in conjunction with patman (which is patch 2/2 of this
series), which can be configured to use a get-maintainer like script to
retrieve the people it should CC based on the patches it can 'git
send-email' for you. If nobody else has a say, I'll push in about a
week.

Thanks for taking a look!

--
Maxim
M
M
Mathieu Othacehe wrote on 27 Dec 2022 11:00
Re: bug#60218: [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87pmc5maj2.fsf_-_@gnu.org
Hey Maxim,

Toggle quote (7 lines)
> cc <team-name> get git send-email flags for cc-ing <team-name>
> cc-members <start> <end> cc teams related to files changed between revisions
> list-teams list teams and their members
> - list-members <team-name> list members belonging to <team-name>~%"))))
> + list-members <team-name> list members belonging to <team-name>~%
> + get-maintainer <patch> compatibility mode with Linux get_maintainer.pl"))))

Maybe it could be interesting to also add this patch mode to the
cc-members command, this way for instance:

cc-members [<start> <end>|<patch> ...]

and also have the get-maintainer command for compatibility with other
tools?

Thanks,

Mathieu
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 1/3] teams: Add a "get-maintainer" command.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-1-maxim.cournoyer@gmail.com
This can be used as a compatibility mode with the get_maintainer.pl Perl
script included in the Linux (or U-Boot) source tree.

* etc/teams.scm.in (git-patch->commit-id): New procedure.
(main) <get-maintainer>: Register new command. Document it.

---

Changes in v2:
- Move newline character (~%) in usage output to the bottom

etc/teams.scm.in | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

Toggle diff (62 lines)
diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index f42a7f6f28..e50efea786 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -5,6 +5,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2022 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2022 Mathieu Othacehe <othacehe@gnu.org>
+;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -34,6 +35,7 @@
(ice-9 format)
(ice-9 regex)
(ice-9 match)
+ (ice-9 rdelim)
(guix ui)
(git))
@@ -623,6 +625,15 @@ (define (diff-revisions rev-start rev-end)
(const 0))
files))
+(define (git-patch->commit-id file)
+ "Parse the commit ID from the first line of FILE, a patch produced with git."
+ (call-with-input-file file
+ (lambda (port)
+ (let ((m (string-match "^From ([0-9a-f]{40})" (read-line port))))
+ (unless m
+ (error "invalid patch file:" file))
+ (match:substring m 1)))))
+
(define (main . args)
(match args
@@ -631,6 +642,12 @@ (define (main . args)
(("cc-members" rev-start rev-end)
(apply cc (find-team-by-scope
(diff-revisions rev-start rev-end))))
+ (("get-maintainer" patch-file)
+ (let* ((rev-end (git-patch->commit-id patch-file))
+ (rev-start (string-append rev-end "^")))
+ (apply main "list-members"
+ (map (compose symbol->string team-id)
+ (find-team-by-scope (diff-revisions rev-start rev-end))))))
(("list-teams" . args)
(list-teams))
(("list-members" . team-names)
@@ -646,6 +663,7 @@ (define (main . args)
cc <team-name> get git send-email flags for cc-ing <team-name>
cc-members <start> <end> cc teams related to files changed between revisions
list-teams list teams and their members
- list-members <team-name> list members belonging to <team-name>~%"))))
+ list-members <team-name> list members belonging to <team-name>
+ get-maintainer <patch> compatibility mode with Linux get_maintainer.pl~%"))))
(apply main (cdr (command-line)))

base-commit: 8f93a1e01a879ae026678dd92c18e2a2a49be540
--
2.38.1
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 2/3] teams: Allow a patch-file argument to cc-members.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-2-maxim.cournoyer@gmail.com
* etc/teams.scm.in (git-patch->revisions): New procedure.
(main) [cc-members]: New match pattern to support patch file argument.
[get-maintainer]: Simplify using the newly introduced procedure from above.
(main): Update usage doc.

---

Changes in v2:
- New: support passing a patch file to the cc-members command

etc/teams.scm.in | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)

Toggle diff (62 lines)
diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index e50efea786..96a04aca3d 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -634,20 +634,29 @@ (define (git-patch->commit-id file)
(error "invalid patch file:" file))
(match:substring m 1)))))
+(define (git-patch->revisions file)
+ "Return the start and end revisions of FILE, a patch file produced with git."
+ (let* ((rev-end (git-patch->commit-id file))
+ (rev-start (string-append rev-end "^")))
+ (list rev-start rev-end)))
+
(define (main . args)
(match args
(("cc" . team-names)
(apply cc (map find-team team-names)))
+ (("cc-members" patch-file)
+ (unless (file-exists? patch-file)
+ (error "patch file does not exist:" patch-file))
+ (apply main "cc-members" (git-patch->revisions patch-file)))
(("cc-members" rev-start rev-end)
(apply cc (find-team-by-scope
(diff-revisions rev-start rev-end))))
(("get-maintainer" patch-file)
- (let* ((rev-end (git-patch->commit-id patch-file))
- (rev-start (string-append rev-end "^")))
- (apply main "list-members"
- (map (compose symbol->string team-id)
- (find-team-by-scope (diff-revisions rev-start rev-end))))))
+ (apply main "list-members"
+ (map (compose symbol->string team-id)
+ (find-team-by-scope (apply diff-revisions
+ (git-patch->revisions patch-file))))))
(("list-teams" . args)
(list-teams))
(("list-members" . team-names)
@@ -660,10 +669,15 @@ (define (main . args)
"Usage: etc/teams.scm <command> [<args>]
Commands:
- cc <team-name> get git send-email flags for cc-ing <team-name>
- cc-members <start> <end> cc teams related to files changed between revisions
- list-teams list teams and their members
- list-members <team-name> list members belonging to <team-name>
- get-maintainer <patch> compatibility mode with Linux get_maintainer.pl~%"))))
+ cc <team-name>
+ get git send-email flags for cc-ing <team-name>
+ cc-members <start> <end> | patch
+ cc teams related to files changed between revisions or in a patch file
+ list-teams
+ list teams and their members
+ list-members <team-name>
+ list members belonging to <team-name>
+ get-maintainer <patch>
+ compatibility mode with Linux get_maintainer.pl~%"))))
(apply main (cdr (command-line)))
--
2.38.1
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 3/3] .patman: New configuration file.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-3-maxim.cournoyer@gmail.com
* .patman: New file.

---

(no changes since v1)

.patman | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 .patman

Toggle diff (17 lines)
diff --git a/.patman b/.patman
new file mode 100644
index 0000000000..4708bb5ea8
--- /dev/null
+++ b/.patman
@@ -0,0 +1,9 @@
+# This config file allows for Patchwork integration with
+# https://patches.guix-patches.cbaines.net/.
+[settings]
+project: guix-patches
+patchwork_url: https://patches.guix-patches.cbaines.net
+add_signoff: False
+# TODO: enable check_patch
+check_patch: False
+get_maintainer_script: etc/teams.scm get-maintainer
--
2.38.1
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:35
Re: bug#60218: [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
8735907tcf.fsf@gmail.com
Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (14 lines)
> Hey Maxim,
>
>> cc <team-name> get git send-email flags for cc-ing <team-name>
>> cc-members <start> <end> cc teams related to files changed between revisions
>> list-teams list teams and their members
>> - list-members <team-name> list members belonging to <team-name>~%"))))
>> + list-members <team-name> list members belonging to <team-name>~%
>> + get-maintainer <patch> compatibility mode with Linux get_maintainer.pl"))))
>
> Maybe it could be interesting to also add this patch mode to the
> cc-members command, this way for instance:
>
> cc-members [<start> <end>|<patch> ...]

Implemented in v2! I guess it could be useful. Note that it doesn't
parse the patch for the file names touched, instead it assumes the patch
was produced from a commit registered in git and then use the usual code
path (for simplicity).

--
Thanks,
Maxim
M
M
Mathieu Othacehe wrote on 28 Dec 2022 18:22
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87v8lvwiid.fsf_-_@gnu.org
Hey,

Toggle quote (5 lines)
> Implemented in v2! I guess it could be useful. Note that it doesn't
> parse the patch for the file names touched, instead it assumes the patch
> was produced from a commit registered in git and then use the usual code
> path (for simplicity).

Nice! I think that it is fair to assume that the commit is registered in
the local git repository.

As a follow-up a documentation update could also be interesting as I
think that the new 'cc-members patch' command is easier to use that the
'cc-member start end' variant.

I had a look to the rest of the patchset it seems fine to me :)

Thanks,

Mathieu
M
M
Maxim Cournoyer wrote on 28 Dec 2022 21:42
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
87a6375kh0.fsf@gmail.com
Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (14 lines)
> Hey,
>
>> Implemented in v2! I guess it could be useful. Note that it doesn't
>> parse the patch for the file names touched, instead it assumes the patch
>> was produced from a commit registered in git and then use the usual code
>> path (for simplicity).
>
> Nice! I think that it is fair to assume that the commit is registered in
> the local git repository.
>
> As a follow-up a documentation update could also be interesting as I
> think that the new 'cc-members patch' command is easier to use that the
> 'cc-member start end' variant.

OK! I intend to document the use of patman along teams.scm, as I find
it helps automate things and keep submissions organized. We can
probably briefly mention the tool, and point the interested user to its
full doc (which lives in u-boot-documentation).

Toggle quote (2 lines)
> I had a look to the rest of the patchset it seems fine to me :)

OK, great! I've now pushed the series. Happy New Year!

--
Thanks,
Maxim
Closed
S
S
Simon Tournier wrote on 6 Jan 2023 18:26
Re: [bug#60218] [PATCH v2 3/3] .patman: New configuration file.
87h6x3h8wh.fsf@gmail.com
Hi Maxim,

On Tue, 27 Dec 2022 at 10:32, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

Toggle quote (12 lines)
> +++ b/.patman
> @@ -0,0 +1,9 @@
> +# This config file allows for Patchwork integration with
> +# https://patches.guix-patches.cbaines.net/.
> +[settings]
> +project: guix-patches
> +patchwork_url: https://patches.guix-patches.cbaines.net
> +add_signoff: False
> +# TODO: enable check_patch
> +check_patch: False
> +get_maintainer_script: etc/teams.scm get-maintainer

Maybe it could be worse to provide an example about how to use this
.patman file. Maybe under Notifying Teams [1] or Teams [2]. WDYT?


Cheers,
simon
Z
Z
zimoun wrote on 11 Jan 2023 16:10
86bkn55cq7.fsf@gmail.com
Hi Maxim,

On Fri, 06 Jan 2023 at 18:26, Simon Tournier <zimon.toutoune@gmail.com> wrote:
Toggle quote (17 lines)
> On Tue, 27 Dec 2022 at 10:32, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> +++ b/.patman
>> @@ -0,0 +1,9 @@
>> +# This config file allows for Patchwork integration with
>> +# https://patches.guix-patches.cbaines.net/.
>> +[settings]
>> +project: guix-patches
>> +patchwork_url: https://patches.guix-patches.cbaines.net
>> +add_signoff: False
>> +# TODO: enable check_patch
>> +check_patch: False
>> +get_maintainer_script: etc/teams.scm get-maintainer
>
> Maybe it could be worse to provide an example about how to use this
> .patman file. Maybe under Notifying Teams [1] or Teams [2]. WDYT?

s/worse/worth :-)

Toggle quote (3 lines)
> 2: https://guix.gnu.org/manual/devel/en/guix.html#Teams

In #58813 [3], you suggest to reference to patman documentation. That’s
my suggestion, so let discuss overthere. :-)


Cheers,
simon
?