diff mbox series

[bug#57646] etc: teams: Add regular expression support to scopes.

Message ID 34c99df65a63bafd7868ad025a85ebb3a4457ac2.camel@gmail.com
State Accepted
Headers show
Series [bug#57646] etc: teams: Add regular expression support to scopes. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Liliana Marie Prikler Sept. 9, 2022, 3:27 p.m. UTC 
* etc/teams.scm (find-teams-by-scope): Differentiate between raw strings
and regexps.  Make raw string matches strict.
---
Hi Mathieu,

this is a fixup to your 1/3 patch, making it so that regexps are supported.
Note, that for the installer team you should now define the scope as
(list "gnu/installer.scm" (make-regexp "^guix/installer/")) or simply
(list (make-regexp "^guix/installer(\\.scm$|/)")).

Cheers

 etc/teams.scm.in | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Mathieu Othacehe Sept. 11, 2022, 4:36 p.m. UTC | #1 
Hey Liliana,

> this is a fixup to your 1/3 patch, making it so that regexps are supported.
> Note, that for the installer team you should now define the scope as
> (list "gnu/installer.scm" (make-regexp "^guix/installer/")) or simply
> (list (make-regexp "^guix/installer(\\.scm$|/)")).

Thanks for the improvement :) Ricardo, any thoughts on this series?

Mathieu
Ricardo Wurmus Sept. 12, 2022, 10:30 a.m. UTC | #2 
Hi Mathieu,

> Thanks for the improvement :) Ricardo, any thoughts on this series?

This looks like a good idea to me, thanks!

Just three comments:

* the dependency on Guile-Git means that the script must be run inside a
  suitable environment now, whereas previously it had no dependencies
  other than Guile.  If we can assume that people use Guix perhaps we
  should do what doc/build.scm does and use Guix to arrange for
  dependencies to be available.

* I don’t like the “if” + “null?” pattern:

+             (if (not (null? (team-scope team)))
+                 (format #f "scope: ~{~s ~}~%" (team-scope team))
+                 ""))

  I’d prefer using match:

  (match (team-scope team)
    (() "")
    (scope (format #f "scope: ~{~s ~}~%" scope)

* With Liliana’s added support for regexes, the previous patch to record
  scopes for the installer etc should be adjusted.
Mathieu Othacehe Sept. 12, 2022, 1:49 p.m. UTC | #3 
Hey,

> * the dependency on Guile-Git means that the script must be run inside a
>   suitable environment now, whereas previously it had no dependencies
>   other than Guile.  If we can assume that people use Guix perhaps we
>   should do what doc/build.scm does and use Guix to arrange for
>   dependencies to be available.

Right, for now I went for the easiest solution and proposed the following
example in the documentation:

--8<---------------cut here---------------start------------->8---
$ guix shell -D guix
[env]$ git send-email $(./etc/teams.scm cc-members HEAD~2 HEAD) *.patch
--8<---------------cut here---------------end--------------->8---

> * I don’t like the “if” + “null?” pattern:
>
> +             (if (not (null? (team-scope team)))
> +                 (format #f "scope: ~{~s ~}~%" (team-scope team))
> +                 ""))
>
>   I’d prefer using match:
>
>   (match (team-scope team)
>     (() "")
>     (scope (format #f "scope: ~{~s ~}~%" scope)
>
> * With Liliana’s added support for regexes, the previous patch to record
>   scopes for the installer etc should be adjusted.

Fixed!

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index 37937a02ff..24664e9c0e 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -32,6 +32,7 @@ 
              (srfi srfi-9)
              (srfi srfi-26)
              (ice-9 format)
+             (ice-9 regex)
              (ice-9 match)
              (guix ui)
              (git))
@@ -281,9 +282,11 @@  (define (find-team-by-scope files)
   (hash-fold
    (lambda (key team acc)
      (if (any (lambda (file)
-                (any (lambda (scope)
-                       ;; XXX: Add regex support?
-                       (string-prefix? scope file))
+                (any (match-lambda
+                       ((? string? scope)
+                        (string=? scope file))
+                       ((? regexp? scope)
+                        (regexp-exec scope file)))
                      (team-scope team)))
               files)
          (cons team acc)