| Message ID | dc3d1c690b40861eab4d623fb8bca88f4194e209.1618502119.git.public@yoctocell.xyz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#47804] lint: Warn about underscores in package names. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
On Thu, 2021-04-15 at 18:00 +0200, Xinglu Chen wrote: > As per section '16.4.2 Package Naming' in the manual, use hyphens > instead of underscores in package names. > > * guix/lint.scm (check-name): Check whether the package name contains > underscores. > --- > There was some discussion about this on guix-devel[1], but no consensus > was reached. Should we whitelist certain names like “86_64” or something? I dunno. Perhaps for now, we can accept that the 'check-name' linter is sometimes overly strict, and punt the exceptions for later? > [1]: https://yhetil.org/guix-devel/87v991vkpi.fsf@nckx > > guix/lint.scm | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/guix/lint.scm b/guix/lint.scm > index a7d6bbba4f..d5ad69475e 100644 > --- a/guix/lint.scm > +++ b/guix/lint.scm > @@ -11,6 +11,7 @@ > ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> > ;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com> > ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com> > +;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz> > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -173,14 +174,20 @@ > (define (check-name package) > "Check whether PACKAGE's name matches our guidelines." > (let ((name (package-name package))) > - ;; Currently checks only whether the name is too short. > - (if (and (<= (string-length name) 1) > - (not (string=? name "r"))) ; common-sense exception > - (list > - (make-warning package > - (G_ "name should be longer than a single character") > - #:field 'name)) > - '()))) > + (cond > + ;; Currently checks only whether the name is too short. > + ((and (<= (string-length name) 1) > + (not (string=? name "r"))) ; common-sense exception > + (list > + (make-warning package > + (G_ "name should be longer than a single character") > + #:field 'name))) > + ((string-match "_" name) > + (list > + (make-warning package > + (G_ "name should use hyphens instead of underscores") > + #:field 'name))) > + (else '())))) I didn't test this, but that seems about right. Ideally, you should add a corresponding test in tests/lint.scm (IIRC, maybe the linter tests are elsewhere). However, Guix is currently in the "string freeze", so this cannot yet be merged, IIUC. Greetings, Maxime.
On Thu, Apr 15 2021, Maxime Devos wrote: >> There was some discussion about this on guix-devel[1], but no >> consensus was reached. Should we whitelist certain names like >> “86_64” or something? > > I dunno. Perhaps for now, we can accept that the 'check-name' linter > is sometimes overly strict, and punt the exceptions for later? Ok, that sounds like a good idea. > I didn't test this, but that seems about right. Ideally, you should > add a corresponding test in tests/lint.scm (IIRC, maybe the linter > tests are elsewhere). Will do. > However, Guix is currently in the "string freeze", so this cannot yet > be merged, IIUC. I thought the “string freeze” was for the manual, or did I miss something? Thanks for the review!
On Thu, 2021-04-15 at 20:15 +0200, Xinglu Chen wrote: > On Thu, Apr 15 2021, Maxime Devos wrote: > [...] > > > However, Guix is currently in the "string freeze", so this cannot yet > > be merged, IIUC. > > I thought the “string freeze” was for the manual, or did I miss > something? <https://lists.gnu.org/archive/html/guix-devel/2021-04/msg00180.html> The freeze if for both the manual and guix itself (but not the packages). Greetings, Maxime.
diff --git a/guix/lint.scm b/guix/lint.scm index a7d6bbba4f..d5ad69475e 100644 --- a/guix/lint.scm +++ b/guix/lint.scm @@ -11,6 +11,7 @@ ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com> ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com> +;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz> ;;; ;;; This file is part of GNU Guix. ;;; @@ -173,14 +174,20 @@ (define (check-name package) "Check whether PACKAGE's name matches our guidelines." (let ((name (package-name package))) - ;; Currently checks only whether the name is too short. - (if (and (<= (string-length name) 1) - (not (string=? name "r"))) ; common-sense exception - (list - (make-warning package - (G_ "name should be longer than a single character") - #:field 'name)) - '()))) + (cond + ;; Currently checks only whether the name is too short. + ((and (<= (string-length name) 1) + (not (string=? name "r"))) ; common-sense exception + (list + (make-warning package + (G_ "name should be longer than a single character") + #:field 'name))) + ((string-match "_" name) + (list + (make-warning package + (G_ "name should use hyphens instead of underscores") + #:field 'name))) + (else '())))) (define (properly-starts-sentence? s) (string-match "^[(\"'`[:upper:][:digit:]]" s))