Re: [PATCH] scripts: add guix lint

[Ludovic Courtès]

Cyril Roelandt <address@hidden> skribis:

> * guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
> * tests/lint.scm: New file.
> * Makefile.am (MODULES, SCM_TESTS): Add them.
> * guix/scripts/build.scm (specification->package): Move from here...
> * guix/ui.scm: ... to here.
> * po/guix/Makevars: Update appropriately.
> * po/guix/POTFILES.in: Update appropriately.
> * doc/guix.texi: Document "guix lint".

Cool!

> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -1459,6 +1459,10 @@ package may actually be built using the @code{guix 
> build} command-line
>  tool (@pxref{Invoking guix build}).  @xref{Packaging Guidelines}, for
>  more information on how to test package definitions.
>  
> +In order to catch common errors and stylistic issues, the packages might
> +actually be checked by running the @command{guix lint} command
> +(@pxref{Invoking guix lint}).

What about just making it like this:

  [...] @xref{Packaging Guidelines}, for more information on how to test
  package definitions, and @ref{Invoking guix lint}, for information
  on how to check a definition for style conformance.

>  Eventually, updating the package definition to a new upstream version
>  can be partly automated by the @command{guix refresh} command
>  (@pxref{Invoking guix refresh}).
> @@ -2326,6 +2330,7 @@ programming interface of Guix in a convenient way.
>  * Invoking guix download::      Downloading a file and printing its hash.
>  * Invoking guix hash::          Computing the cryptographic hash of a file.
>  * Invoking guix refresh::       Updating package definitions.
> +* Invoking guix lint::          Finding errors in package definitions.
>  @end menu
>  
>  @node Invoking guix build
> @@ -2703,6 +2708,28 @@ for in @code{$PATH}.
>  
>  @end table
>  
> address@hidden Invoking guix lint
> address@hidden Invoking @command{guix lint}
> +The @command{guix lint} command runs a few checks on a given set of
> +packages in order to find common mistakes in their definitions.

To make the target audience clearer, what about this:

  The @command{guix lint} is meant to help package developers avoid
  common errors and use a consistent style.  It runs a series of checks
  on the given set of packages [...]

> -(define (specification->package spec)
> -  "Return a package matching SPEC.  SPEC may be a package name, or a package
> -name followed by a hyphen and a version number.  If the version number is not
> -present, return the preferred newest version."
> -  (let-values (((name version)
> -                (package-name->name+version spec)))
> -    (match (find-best-packages-by-name name version)

Apologies for giving incorrect advice in my previous review; could you
move this procedure to (gnu packages) (and not (guix ui), as I
wrongfully suggested)?  Ideally in a separate commit.

The reason is to avoid introducing a dependency from (guix ui) to (gnu
packages).

OK to commit with these minor issues fixed.

Thanks!

Ludo’.