guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#26312: [PATCH] gnu: Add cifs-utils.


From: Tobias Geerinckx-Rice
Subject: bug#26312: [PATCH] gnu: Add cifs-utils.
Date: Thu, 30 Mar 2017 22:19:54 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

Thomas,

I don't have any Samba shares to test this on, but will try my hand at
this reviewing business anyway.

On 30/03/17 17:48, Thomas Danckaert wrote:
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'autoreconf
> +           (lambda _ ; install.sh is missing from release tarball
> +             (zero? (system* "autoreconf" "-i"))))

If it's a one-time oversight, best leave a note for the next update:

            ;; The 6.7 tarball is missing ‘install.sh’. Create it.

(Your shorter in-line form is fine too; I just think active comments are
more clear.)

I noticed that the ‘--enable-systemd’ configure flag is enabled by
default, but it seems to be well-behaved on systems without a running
systemd.

> +    (synopsis "User-space utilities for CIFS (Samba) mounts")
> +    (description "@code{cifs-utils} is a set of user-space tools used
> +by the in-kernel CIFS filesystem.")

If this package is as Linux-specific at it seems, I'd explicitly mention
that in both the synopsis and description.

Aside: GNU uses ‘file system’, not ‘filesystem’, because GNU is rad. How
about expanding the acronym at the same time? E.g.:

  ‘the @{Common Internet File System} (CIFS) implementation built into
   the Linux kernel’.

Bonus points for replacing the horrid word ‘implementation’ :-)

Thanks!

T G-R

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]