Subject: Re: [bug#48630] [PATCH] adds `ack' package and it's perl module dependency File::Next

From: Xinglu Chen <[email-masked]>
Date: Mon, 24 May 2021 23:26:22 +0200

On Mon, May 24 2021, Gabriel Wicki wrote:

> modified:   gnu/packages/perl.scm
>  - adds package `perl-file-next' (module File::Next)
>
> modified:   gnu/packages/textutils.scm
>  - adds package `ack' with disabled tests

Commit messages should be written in ChangeLog format[1], you can look
at previous commits for examples.

This commit should also be split into two separate commits, one adding
‘perl-file-next’, and the other adding ‘ack’.

> +(define-public perl-file-next
> +  (package
> +   (name "perl-file-next")
> +   (version "1.18")
> +   (source (origin
> +            (method git-fetch)
> +            (uri (git-reference
> +                  (url "https://github.com/petdance/file-next")
> +                  (commit version)))
> +            (file-name "perl-next-file")

Use ‘(file-name (git-file-name name version))’.

> +            (sha256
> +             (base32
> +              "0zdrxk409qxkbbv4fl4wi285kfzyrpaja9wfl00vrxc078rs4afm"))))
> +   (build-system perl-build-system)
> +   (synopsis "File::Next is a lightweight, taint-safe file-finding Perl module")

Try to make the synopsis as concise as possible, just “Lightweight,
taint-safe file-finding Perl module” should do.  See the ‘Synopses and
Descriptions’ section of the manual.

> +   (description "A Perl CPAN module for finding files.  It has no non-core
> +prerequisites")

The description should contain one or more complete sentences.

> +(define-public ack
> +  (package
> +   (name "ack")
> +   (version "3.5.0")
> +   (source (origin
> +            (method git-fetch)
> +            (uri
> +             (git-reference
> +              (url "https://github.com/beyondgrep/ack3")
> +              (commit (format #nil "v~a" version))))

We usually use ‘(string-append "v" version)’ instead of ‘format’.

> +            (file-name "ack")

Use ‘(file-name (git-file-name name version))’ (same as above).

> +            (sha256
> +             (base32 "00131vqjbzcn6w22m0h3j6x9kp59dimfnnqhpmi78vbcj0jws1dv"))))
> +   (build-system perl-build-system)
> +   (arguments '(#:tests? #f))

What’s the reason for disabling the tests?

> +   (propagated-inputs `(("perl-file-next" ,perl-file-next)))
> +   (synopsis "Code-searching tool optimized for programmers with large trees
> +of source code")
> +   (description "ack is a tool for finding text inside files. it is designed for
                                                                ^^
Please use double spacing, and capitalize.

> +hackers and programmers by being fast, ignoring VCS directories, letting a user
> +easily specify file types, match highlighting, Perl-Compatible Regular
> +Expressions, and being faster to type than `grep '")
                                                   ^^
Unecessary whitespace, and missing period.

Could you send an updated series?

[1]: https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs