| Message ID | 20221220141330.30372-1-maxim.cournoyer@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | New teams.scm 'get-maintainer' command (for integration with patman) | expand |
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. > + (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.
Hi Ricardo, Ricardo Wurmus <rekado@elephly.net> writes: > 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. >> + (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. > 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!
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> ...] and also have the get-maintainer command for compatibility with other tools? Thanks, Mathieu
Hi Mathieu, Mathieu Othacehe <othacehe@gnu.org> writes: > 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).
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. I had a look to the rest of the patchset it seems fine to me :) Thanks, Mathieu
Hi Mathieu, Mathieu Othacehe <othacehe@gnu.org> writes: > 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). > 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!
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)))