unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH 0/5] Add, fix and improve checkers in guix lint.
@ 2014-09-27 21:49 Cyril Roelandt
  2014-09-27 21:49 ` [PATCH 1/5] guix lint: make sure check-patches retrieves patch names Cyril Roelandt
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

The following five patches improve the "guix lint" command, according to
suggestions from Alex Kost and Ludovic Courtès.


Cyril Roelandt (5):
  guix lint: make sure check-patches retrieves patch names.
  guix lint: Make sure synopses are not too long.
  guix lint: check whether descriptions and synopses start with an
    upper-case letter.
  guix lint: Make sure a synopsis cannot start with a lower-case
    article.
  guix lint: make sure synopses do not start with the package name.

 guix/scripts/lint.scm | 49 ++++++++++++++++++++++++++++++++++++++++++---
 tests/lint.scm        | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 3 deletions(-)

-- 
1.8.4.rc3

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] guix lint: make sure check-patches retrieves patch names.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
@ 2014-09-27 21:49 ` Cyril Roelandt
  2014-09-28 10:08   ` Ludovic Courtès
  2014-09-27 21:49 ` [PATCH 2/5] guix lint: Make sure synopses are not too long Cyril Roelandt
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

* guix/lint (check-patches): Test the output of origin-patches.
---
 guix/scripts/lint.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 83dde9a..0fbec1b 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -152,6 +152,7 @@ Run a set of checkers on the specified package; if none is specified, run the ch
         (name      (package-name package))
         (full-name (package-full-name package)))
     (if (and patches
+             (every string? patches)
              (any (lambda (patch)
                     (let ((filename (basename patch)))
                       (not (or (eq? (string-contains filename name) 0)
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] guix lint: Make sure synopses are not too long.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
  2014-09-27 21:49 ` [PATCH 1/5] guix lint: make sure check-patches retrieves patch names Cyril Roelandt
@ 2014-09-27 21:49 ` Cyril Roelandt
  2014-09-28 10:09   ` Ludovic Courtès
  2014-09-27 21:49 ` [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter Cyril Roelandt
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

* guix/scripts/lint.scm (check-synopsis-length): New procedure.
* tests/lint.scm: test it.
---
 guix/scripts/lint.scm | 9 ++++++++-
 tests/lint.scm        | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 0fbec1b..12391ef 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -140,11 +140,18 @@ Run a set of checkers on the specified package; if none is specified, run the ch
                      "no article allowed at the beginning of the synopsis"
                      'synopsis)))
 
+  (define (check-synopsis-length synopsis)
+   (if (>= (string-length synopsis) 80)
+       (emit-warning package
+                     "synopsis should be less than 80 characters long"
+                     'synopsis)))
+
  (let ((synopsis (package-synopsis package)))
    (if (string? synopsis)
        (begin
         (check-final-period synopsis)
-        (check-start-article synopsis)))))
+        (check-start-article synopsis)
+        (check-synopsis-length synopsis)))))
 
 (define (check-patches package)
   ;; Emit a warning if the patches requires by PACKAGE are badly named.
diff --git a/tests/lint.scm b/tests/lint.scm
index 56558c9..e082908 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -79,6 +79,15 @@
                         (check-synopsis-style pkg))))
                     "no article allowed at the beginning of the synopsis")))
 
+(test-assert "synopsis: too long"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (synopsis (make-string 80 #\x)))))
+                          (check-synopsis-style pkg))))
+                    "synopsis should be less than 80 characters long")))
+
 (test-assert "inputs: pkg-config is probably a native input"
   (->bool
    (string-contains
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
  2014-09-27 21:49 ` [PATCH 1/5] guix lint: make sure check-patches retrieves patch names Cyril Roelandt
  2014-09-27 21:49 ` [PATCH 2/5] guix lint: Make sure synopses are not too long Cyril Roelandt
@ 2014-09-27 21:49 ` Cyril Roelandt
  2014-09-28 16:09   ` Ludovic Courtès
  2014-09-27 21:49 ` [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article Cyril Roelandt
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

* guix/scripts/lint.scm (check-description-style, check-synopsis-start-upper-case): New methods
* tests/lint.scm: Test them.
---
 guix/scripts/lint.scm | 26 ++++++++++++++++++++++++++
 tests/lint.scm        | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 12391ef..5f9b6e4 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -29,6 +29,7 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-37)
   #:export (guix-lint
+            check-description-style
             check-inputs-should-be-native
             check-patches
             check-synopsis-style))
@@ -110,6 +111,19 @@ Run a set of checkers on the specified package; if none is specified, run the ch
             %checkers)
   (exit 0))
 
+(define (start-with-upper-case s)
+  (char-set-contains? char-set:upper-case (string-ref s 0)))
+
+(define (check-description-style package)
+  ;; Emit a warning if stylistic issues are found in the description of PACKAGE.
+ (let ((description (package-description package)))
+   (when (and (string? description)
+              (not (string-null? description))
+              (not (start-with-upper-case description)))
+     (emit-warning package
+                   "description should start with an upper-case letter"
+                   'description))))
+
 (define (check-inputs-should-be-native package)
   ;; Emit a warning if some inputs of PACKAGE are likely to belong to its
   ;; native inputs.
@@ -146,9 +160,17 @@ Run a set of checkers on the specified package; if none is specified, run the ch
                      "synopsis should be less than 80 characters long"
                      'synopsis)))
 
+  (define (check-synopsis-start-upper-case synopsis)
+   (when (and (not (string-null? synopsis))
+              (not (start-with-upper-case synopsis)))
+     (emit-warning package
+                   "synopsis should start with an upper-case letter"
+                   'synopsis)))
+
  (let ((synopsis (package-synopsis package)))
    (if (string? synopsis)
        (begin
+        (check-synopsis-start-upper-case synopsis)
         (check-final-period synopsis)
         (check-start-article synopsis)
         (check-synopsis-length synopsis)))))
@@ -172,6 +194,10 @@ Run a set of checkers on the specified package; if none is specified, run the ch
 (define %checkers
   (list
    (lint-checker
+     (name        "description")
+     (description "Validate package descriptions")
+     (check       check-description-style))
+   (lint-checker
      (name        "inputs-should-be-native")
      (description "Identify inputs that should be native inputs")
      (check       check-inputs-should-be-native))
diff --git a/tests/lint.scm b/tests/lint.scm
index e082908..ceb7abe 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -44,6 +44,24 @@
       (thunk))
     (get-output-string port)))
 
+(test-assert "description: does not start with an upper-case letter"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (description "bad description."))))
+                          (check-description-style pkg))))
+                    "description should start with an upper-case letter")))
+
+(test-assert "synopsis: does not start with an upper-case letter"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (synopsis "bad synopsis."))))
+                          (check-synopsis-style pkg))))
+                    "synopsis should start with an upper-case letter")))
+
 (test-assert "synopsis: ends with a period"
   (->bool
    (string-contains (call-with-warnings
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
                   ` (2 preceding siblings ...)
  2014-09-27 21:49 ` [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter Cyril Roelandt
@ 2014-09-27 21:49 ` Cyril Roelandt
  2014-09-28 16:11   ` Ludovic Courtès
  2014-09-27 21:49 ` [PATCH 5/5] guix lint: make sure synopses do not start with the package name Cyril Roelandt
  2014-09-28 16:49 ` [PATCH 0/5] Add, fix and improve checkers in guix lint Andreas Enge
  5 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

* guix/scripts/lint.scm (check-start-article): use "string-ci=?" instead of "string=?".
* tests/lint.scm: Add corresponding tests.
---
 guix/scripts/lint.scm |  4 ++--
 tests/lint.scm        | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 5f9b6e4..ee00e5c 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -148,8 +148,8 @@ Run a set of checkers on the specified package; if none is specified, run the ch
                       'synopsis)))
 
   (define (check-start-article synopsis)
-   (if (or (string=? (string-take synopsis 2) "A ")
-           (string=? (string-take synopsis 3) "An "))
+   (if (or (string-ci=? (string-take synopsis 2) "A ")
+           (string-ci=? (string-take synopsis 3) "An "))
        (emit-warning package
                      "no article allowed at the beginning of the synopsis"
                      'synopsis)))
diff --git a/tests/lint.scm b/tests/lint.scm
index ceb7abe..c439faa 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -97,6 +97,24 @@
                         (check-synopsis-style pkg))))
                     "no article allowed at the beginning of the synopsis")))
 
+(test-assert "synopsis: starts with 'a'"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (synopsis "a bad synopsis"))))
+                        (check-synopsis-style pkg))))
+                    "no article allowed at the beginning of the synopsis")))
+
+(test-assert "synopsis: starts with 'an'"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (synopsis "an awful synopsis"))))
+                        (check-synopsis-style pkg))))
+                    "no article allowed at the beginning of the synopsis")))
+
 (test-assert "synopsis: too long"
   (->bool
    (string-contains (call-with-warnings
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] guix lint: make sure synopses do not start with the package name.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
                   ` (3 preceding siblings ...)
  2014-09-27 21:49 ` [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article Cyril Roelandt
@ 2014-09-27 21:49 ` Cyril Roelandt
  2014-09-28 16:11   ` Ludovic Courtès
  2014-09-28 16:49 ` [PATCH 0/5] Add, fix and improve checkers in guix lint Andreas Enge
  5 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-09-27 21:49 UTC (permalink / raw)
  To: guix-devel

* guix/scripts/lint.scm (check-start-with-package-name): New method.
* tests/lint.scm: Test it.
---
 guix/scripts/lint.scm |  9 +++++++++
 tests/lint.scm        | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index ee00e5c..8f49afc 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -167,12 +167,21 @@ Run a set of checkers on the specified package; if none is specified, run the ch
                    "synopsis should start with an upper-case letter"
                    'synopsis)))
 
+  (define (check-start-with-package-name synopsis)
+   (let ((idx (string-contains-ci synopsis (package-name package))))
+     (when (and idx
+                (= idx 0))
+       (emit-warning package
+                     "synopsis should not start with the package name")
+                     'synopsis)))
+
  (let ((synopsis (package-synopsis package)))
    (if (string? synopsis)
        (begin
         (check-synopsis-start-upper-case synopsis)
         (check-final-period synopsis)
         (check-start-article synopsis)
+        (check-start-with-package-name synopsis)
         (check-synopsis-length synopsis)))))
 
 (define (check-patches package)
diff --git a/tests/lint.scm b/tests/lint.scm
index c439faa..62a9df9 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -124,6 +124,16 @@
                           (check-synopsis-style pkg))))
                     "synopsis should be less than 80 characters long")))
 
+(test-assert "synopsis: start with package name"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (name "foo")
+                                     (synopsis "foo, a nice package"))))
+                          (check-synopsis-style pkg))))
+                    "synopsis should not start with the package name")))
+
 (test-assert "inputs: pkg-config is probably a native input"
   (->bool
    (string-contains
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] guix lint: make sure check-patches retrieves patch names.
  2014-09-27 21:49 ` [PATCH 1/5] guix lint: make sure check-patches retrieves patch names Cyril Roelandt
@ 2014-09-28 10:08   ` Ludovic Courtès
  2014-10-06  0:26     ` [PATCHv2 " Cyril Roelandt
  0 siblings, 1 reply; 20+ messages in thread
From: Ludovic Courtès @ 2014-09-28 10:08 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/lint (check-patches): Test the output of origin-patches.
> ---
>  guix/scripts/lint.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index 83dde9a..0fbec1b 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -152,6 +152,7 @@ Run a set of checkers on the specified package; if none is specified, run the ch
>          (name      (package-name package))
>          (full-name (package-full-name package)))
>      (if (and patches
> +             (every string? patches)
>               (any (lambda (patch)
>                      (let ((filename (basename patch)))
>                        (not (or (eq? (string-contains filename name) 0)

Instead of requiring every item in ‘patches’ to be a file name, what
about testing each item individually:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1050 bytes --]

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 83dde9a..b613737 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -152,10 +152,15 @@ Run a set of checkers on the specified package; if none is specified, run the ch
         (name      (package-name package))
         (full-name (package-full-name package)))
     (if (and patches
-             (any (lambda (patch)
+             (any (match-lambda
+                   ((? string? patch)
                     (let ((filename (basename patch)))
                       (not (or (eq? (string-contains filename name) 0)
-                               (eq? (string-contains filename full-name) 0)))))
+                               (eq? (string-contains filename full-name)
+                                    0)))))
+                   (_
+                    ;; This must be an <origin> or something like that.
+                    #f))
                   patches))
         (emit-warning package
           "file names of patches should start with the package name"

[-- Attachment #3: Type: text/plain, Size: 21 bytes --]


Thanks,
Ludo’.

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] guix lint: Make sure synopses are not too long.
  2014-09-27 21:49 ` [PATCH 2/5] guix lint: Make sure synopses are not too long Cyril Roelandt
@ 2014-09-28 10:09   ` Ludovic Courtès
  0 siblings, 0 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-09-28 10:09 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/scripts/lint.scm (check-synopsis-length): New procedure.
> * tests/lint.scm: test it.

Last line should be:

  * tests/lint.scm ("synopsis: too long"): New test.

OK to push with that!

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-09-27 21:49 ` [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter Cyril Roelandt
@ 2014-09-28 16:09   ` Ludovic Courtès
  2014-10-06  0:27     ` [PATCHv2 " Cyril Roelandt
  2014-10-06  0:28     ` [PATCH " Cyril Roelandt
  0 siblings, 2 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-09-28 16:09 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/scripts/lint.scm (check-description-style, check-synopsis-start-upper-case): New methods
> * tests/lint.scm: Test them.

Please specify the test names, and wrap lines.

> +(define (start-with-upper-case s)
> +  (char-set-contains? char-set:upper-case (string-ref s 0)))

Rather ‘starts-with-capital-letter?’ (with the question mark, as is
conventional, and more grammatically correct I think.)

Also it may be worth checking that S has one or more character, so
gracefully handle empty synopses.

OK to push with these changes, thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article.
  2014-09-27 21:49 ` [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article Cyril Roelandt
@ 2014-09-28 16:11   ` Ludovic Courtès
  2014-10-06  0:28     ` Cyril Roelandt
  0 siblings, 1 reply; 20+ messages in thread
From: Ludovic Courtès @ 2014-09-28 16:11 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/scripts/lint.scm (check-start-article): use "string-ci=?" instead of "string=?".
> * tests/lint.scm: Add corresponding tests.

Test names, lines wrapped please.  :-)

>    (define (check-start-article synopsis)
> -   (if (or (string=? (string-take synopsis 2) "A ")
> -           (string=? (string-take synopsis 3) "An "))
> +   (if (or (string-ci=? (string-take synopsis 2) "A ")
> +           (string-ci=? (string-take synopsis 3) "An "))

There’s also a problem if SYNOPSIS is shorter, but I wonder if we should
worry about it?

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] guix lint: make sure synopses do not start with the package name.
  2014-09-27 21:49 ` [PATCH 5/5] guix lint: make sure synopses do not start with the package name Cyril Roelandt
@ 2014-09-28 16:11   ` Ludovic Courtès
  0 siblings, 0 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-09-28 16:11 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/scripts/lint.scm (check-start-with-package-name): New method.
> * tests/lint.scm: Test it.

Test name please, and OK to commit.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/5] Add, fix and improve checkers in guix lint.
  2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
                   ` (4 preceding siblings ...)
  2014-09-27 21:49 ` [PATCH 5/5] guix lint: make sure synopses do not start with the package name Cyril Roelandt
@ 2014-09-28 16:49 ` Andreas Enge
  2014-10-04 14:25   ` Cyril Roelandt
  5 siblings, 1 reply; 20+ messages in thread
From: Andreas Enge @ 2014-09-28 16:49 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

On Sat, Sep 27, 2014 at 11:49:23PM +0200, Cyril Roelandt wrote:
>   guix lint: check whether descriptions and synopses start with an
>     upper-case letter.
>   guix lint: Make sure a synopsis cannot start with a lower-case
>     article.

Is the second test not redundant?

Andreas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/5] Add, fix and improve checkers in guix lint.
  2014-09-28 16:49 ` [PATCH 0/5] Add, fix and improve checkers in guix lint Andreas Enge
@ 2014-10-04 14:25   ` Cyril Roelandt
  0 siblings, 0 replies; 20+ messages in thread
From: Cyril Roelandt @ 2014-10-04 14:25 UTC (permalink / raw)
  To: Andreas Enge; +Cc: guix-devel

On 09/28/2014 06:49 PM, Andreas Enge wrote:
> On Sat, Sep 27, 2014 at 11:49:23PM +0200, Cyril Roelandt wrote:
>>   guix lint: check whether descriptions and synopses start with an
>>     upper-case letter.
>>   guix lint: Make sure a synopsis cannot start with a lower-case
>>     article.
> 
> Is the second test not redundant?
>

Well, sort of, but not really. If you start a synopsis by "an", guix
lint should issue two warnings (the synopsys starts with a lowercase
letter and with an article), so I think it's fine.


Cyril.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCHv2 1/5] guix lint: make sure check-patches retrieves patch names.
  2014-09-28 10:08   ` Ludovic Courtès
@ 2014-10-06  0:26     ` Cyril Roelandt
  2014-10-06 15:57       ` Ludovic Courtès
  0 siblings, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-10-06  0:26 UTC (permalink / raw)
  To: guix-devel

* guix/lint (check-patches): Test the output of origin-patches.
---
 guix/scripts/lint.scm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 83dde9a..b613737 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -152,10 +152,15 @@ Run a set of checkers on the specified package; if none is specified, run the ch
         (name      (package-name package))
         (full-name (package-full-name package)))
     (if (and patches
-             (any (lambda (patch)
+             (any (match-lambda
+                   ((? string? patch)
                     (let ((filename (basename patch)))
                       (not (or (eq? (string-contains filename name) 0)
-                               (eq? (string-contains filename full-name) 0)))))
+                               (eq? (string-contains filename full-name)
+                                    0)))))
+                   (_
+                    ;; This must be an <origin> or something like that.
+                    #f))
                   patches))
         (emit-warning package
           "file names of patches should start with the package name"
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCHv2 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-09-28 16:09   ` Ludovic Courtès
@ 2014-10-06  0:27     ` Cyril Roelandt
  2014-10-06 16:00       ` Ludovic Courtès
  2014-10-06  0:28     ` [PATCH " Cyril Roelandt
  1 sibling, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-10-06  0:27 UTC (permalink / raw)
  To: guix-devel

* guix/scripts/lint.scm (check-description-style,
  check-synopsis-start-upper-case): New methods.
* tests/lint.scm ("description: does not start with an upper-case letter",
  "synopsis: does not start with an upper-case letter"): New tests.
---
 guix/scripts/lint.scm | 26 ++++++++++++++++++++++++++
 tests/lint.scm        | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index e8d3707..1124654 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -29,6 +29,7 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-37)
   #:export (guix-lint
+            check-description-style
             check-inputs-should-be-native
             check-patches
             check-synopsis-style))
@@ -110,6 +111,19 @@ Run a set of checkers on the specified package; if none is specified, run the ch
             %checkers)
   (exit 0))
 
+(define (start-with-capital-letter? s)
+  (char-set-contains? char-set:upper-case (string-ref s 0)))
+
+(define (check-description-style package)
+  ;; Emit a warning if stylistic issues are found in the description of PACKAGE.
+ (let ((description (package-description package)))
+   (when (and (string? description)
+              (not (string-null? description))
+              (not (start-with-capital-letter? description)))
+     (emit-warning package
+                   "description should start with an upper-case letter"
+                   'description))))
+
 (define (check-inputs-should-be-native package)
   ;; Emit a warning if some inputs of PACKAGE are likely to belong to its
   ;; native inputs.
@@ -146,9 +160,17 @@ Run a set of checkers on the specified package; if none is specified, run the ch
                      "synopsis should be less than 80 characters long"
                      'synopsis)))
 
+  (define (check-synopsis-start-upper-case synopsis)
+   (when (and (not (string-null? synopsis))
+              (not (start-with-capital-letter? synopsis)))
+     (emit-warning package
+                   "synopsis should start with an upper-case letter"
+                   'synopsis)))
+
  (let ((synopsis (package-synopsis package)))
    (if (string? synopsis)
        (begin
+        (check-synopsis-start-upper-case synopsis)
         (check-final-period synopsis)
         (check-start-article synopsis)
         (check-synopsis-length synopsis)))))
@@ -176,6 +198,10 @@ Run a set of checkers on the specified package; if none is specified, run the ch
 (define %checkers
   (list
    (lint-checker
+     (name        "description")
+     (description "Validate package descriptions")
+     (check       check-description-style))
+   (lint-checker
      (name        "inputs-should-be-native")
      (description "Identify inputs that should be native inputs")
      (check       check-inputs-should-be-native))
diff --git a/tests/lint.scm b/tests/lint.scm
index e082908..ceb7abe 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -44,6 +44,24 @@
       (thunk))
     (get-output-string port)))
 
+(test-assert "description: does not start with an upper-case letter"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (description "bad description."))))
+                          (check-description-style pkg))))
+                    "description should start with an upper-case letter")))
+
+(test-assert "synopsis: does not start with an upper-case letter"
+  (->bool
+   (string-contains (call-with-warnings
+                      (lambda ()
+                        (let ((pkg (dummy-package "x"
+                                     (synopsis "bad synopsis."))))
+                          (check-synopsis-style pkg))))
+                    "synopsis should start with an upper-case letter")))
+
 (test-assert "synopsis: ends with a period"
   (->bool
    (string-contains (call-with-warnings
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-09-28 16:09   ` Ludovic Courtès
  2014-10-06  0:27     ` [PATCHv2 " Cyril Roelandt
@ 2014-10-06  0:28     ` Cyril Roelandt
  2014-10-06 15:58       ` Ludovic Courtès
  1 sibling, 1 reply; 20+ messages in thread
From: Cyril Roelandt @ 2014-10-06  0:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On 09/28/2014 06:09 PM, Ludovic Courtès wrote:
> Cyril Roelandt <tipecaml@gmail.com> skribis:
> 
>> * guix/scripts/lint.scm (check-description-style, check-synopsis-start-upper-case): New methods
>> * tests/lint.scm: Test them.
> 
> Please specify the test names, and wrap lines.
> 
>> +(define (start-with-upper-case s)
>> +  (char-set-contains? char-set:upper-case (string-ref s 0)))
> 
> Rather ‘starts-with-capital-letter?’ (with the question mark, as is
> conventional, and more grammatically correct I think.)
> 
> Also it may be worth checking that S has one or more character, so
> gracefully handle empty synopses.
> 
Not sure whether the "starts-with-capital-letter" function would make a
lot of sense with an empty string, that why I test this in the caller.
WDYT ?


> OK to push with these changes, thanks!
> 
> Ludo’.
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article.
  2014-09-28 16:11   ` Ludovic Courtès
@ 2014-10-06  0:28     ` Cyril Roelandt
  0 siblings, 0 replies; 20+ messages in thread
From: Cyril Roelandt @ 2014-10-06  0:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On 09/28/2014 06:11 PM, Ludovic Courtès wrote:
> Cyril Roelandt <tipecaml@gmail.com> skribis:
> 
>> * guix/scripts/lint.scm (check-start-article): use "string-ci=?" instead of "string=?".
>> * tests/lint.scm: Add corresponding tests.
> 
> Test names, lines wrapped please.  :-)
> 
>>    (define (check-start-article synopsis)
>> -   (if (or (string=? (string-take synopsis 2) "A ")
>> -           (string=? (string-take synopsis 3) "An "))
>> +   (if (or (string-ci=? (string-take synopsis 2) "A ")
>> +           (string-ci=? (string-take synopsis 3) "An "))
> 
> There’s also a problem if SYNOPSIS is shorter, but I wonder if we should
> worry about it?
> 

Nah, doesn't seem so bad.

Cyril.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCHv2 1/5] guix lint: make sure check-patches retrieves patch names.
  2014-10-06  0:26     ` [PATCHv2 " Cyril Roelandt
@ 2014-10-06 15:57       ` Ludovic Courtès
  0 siblings, 0 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-10-06 15:57 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/lint (check-patches): Test the output of origin-patches.

OK, please push.

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-10-06  0:28     ` [PATCH " Cyril Roelandt
@ 2014-10-06 15:58       ` Ludovic Courtès
  0 siblings, 0 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-10-06 15:58 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> On 09/28/2014 06:09 PM, Ludovic Courtès wrote:
>> Cyril Roelandt <tipecaml@gmail.com> skribis:
>> 
>>> * guix/scripts/lint.scm (check-description-style, check-synopsis-start-upper-case): New methods
>>> * tests/lint.scm: Test them.
>> 
>> Please specify the test names, and wrap lines.
>> 
>>> +(define (start-with-upper-case s)
>>> +  (char-set-contains? char-set:upper-case (string-ref s 0)))
>> 
>> Rather ‘starts-with-capital-letter?’ (with the question mark, as is
>> conventional, and more grammatically correct I think.)
>> 
>> Also it may be worth checking that S has one or more character, so
>> gracefully handle empty synopses.
>> 
> Not sure whether the "starts-with-capital-letter" function would make a
> lot of sense with an empty string, that why I test this in the caller.
> WDYT ?

Oh right, I hadn’t noticed the test in the caller, sorry about that.

Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCHv2 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter.
  2014-10-06  0:27     ` [PATCHv2 " Cyril Roelandt
@ 2014-10-06 16:00       ` Ludovic Courtès
  0 siblings, 0 replies; 20+ messages in thread
From: Ludovic Courtès @ 2014-10-06 16:00 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/scripts/lint.scm (check-description-style,
>   check-synopsis-start-upper-case): New methods.
> * tests/lint.scm ("description: does not start with an upper-case letter",
>   "synopsis: does not start with an upper-case letter"): New tests.

LGTM, please push.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-10-06 16:00 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-27 21:49 [PATCH 0/5] Add, fix and improve checkers in guix lint Cyril Roelandt
2014-09-27 21:49 ` [PATCH 1/5] guix lint: make sure check-patches retrieves patch names Cyril Roelandt
2014-09-28 10:08   ` Ludovic Courtès
2014-10-06  0:26     ` [PATCHv2 " Cyril Roelandt
2014-10-06 15:57       ` Ludovic Courtès
2014-09-27 21:49 ` [PATCH 2/5] guix lint: Make sure synopses are not too long Cyril Roelandt
2014-09-28 10:09   ` Ludovic Courtès
2014-09-27 21:49 ` [PATCH 3/5] guix lint: check whether descriptions and synopses start with an upper-case letter Cyril Roelandt
2014-09-28 16:09   ` Ludovic Courtès
2014-10-06  0:27     ` [PATCHv2 " Cyril Roelandt
2014-10-06 16:00       ` Ludovic Courtès
2014-10-06  0:28     ` [PATCH " Cyril Roelandt
2014-10-06 15:58       ` Ludovic Courtès
2014-09-27 21:49 ` [PATCH 4/5] guix lint: Make sure a synopsis cannot start with a lower-case article Cyril Roelandt
2014-09-28 16:11   ` Ludovic Courtès
2014-10-06  0:28     ` Cyril Roelandt
2014-09-27 21:49 ` [PATCH 5/5] guix lint: make sure synopses do not start with the package name Cyril Roelandt
2014-09-28 16:11   ` Ludovic Courtès
2014-09-28 16:49 ` [PATCH 0/5] Add, fix and improve checkers in guix lint Andreas Enge
2014-10-04 14:25   ` Cyril Roelandt

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).