[bug#47364] [PATCH 2/2] services: slim: Add pam-gnupg support.

[Maxime Devos <maximedevos@telenet.be>]

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

Hi,

I'm not familiar with PAM, so I can't do much reviewing about that
(seems ok, though I'm no expert).  Some nitpicks:

On Wed, 2021-03-24 at 19:52 +0300, Oleg Pykhalov wrote:
> [...]
> 
> diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
> index ad02586be8..75edd01908 100644
> --- a/gnu/system/pam.scm
> +++ b/gnu/system/pam.scm
> [...]
>     (lambda* (name #:key allow-empty-passwords? (allow-root? #f) motd
> -                   login-uid?)
> +                   login-uid? (gnupg? #f))

Nitpick: keyword variables have #f as default by default, so you could just write ...

     (lambda* (name #:key allow-empty-passwords? (allow-root? #f) motd
> -                   login-uid?)
> +                   login-uid? gnupg?)

... here.  As a minimal example, you could run the following code in a Guile REPL:

> ;; These both evaluate to (#f #f)!
> ((lambda* (#:key login-uid? gnupg?) (list login-uid? gnupg?)))
> ((lambda* (#:key login-uid? (gnupg? #f)) (list login-uid? gnupg?)))

Hmm, maybe (allow-root? #f) could be replaced with simply allow-root? here ...

>        "Return a standard Unix-style PAM service for NAME.  When
>  ALLOW-EMPTY-PASSWORDS? is true, allow empty passwords.  When ALLOW-ROOT? is
>  true, allow root to run the command without authentication.  When MOTD is

It would be nice if this docstring documents GNUPG? as well.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]