[bug#74819] [PATCH v3] services: elogind: Support Hook Directories

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

45mg <45mg.writes@gmail.com> skribis:

> Allow the user to specify scripts to be added into Elogind's hook
> directories. These scripts will be run before/after
> suspend/hibernate/poweroff/reboot.
>
> Also allow setting the associated config options.
>
> * gnu/services/desktop.scm (elogind-configuration): add
> `system-sleep-hook-files`, `system-shutdown-hook-files`,
> and 4 new config options.
> (elogind-configuration-file): Add entries for the new config options
> under the `[Sleep]` section.
> (/etc/elogind): New function, to generate /etc/elogind directory.
> (elogind-service-type): Extend `etc-service-type` using `/etc/elogind`.
> * doc/guix.texi: Document the new options.
>
> Change-Id: I7e22cbaa9d031049b9d085ba0ce4cc8a8b4f16ff

Nice!


[...]

> +@item @code{allow-power-off-interrupts?} (default: @code{#f}) (type: boolean)
> +@item @code{allow-suspend-interrupts?} (default: @code{#f}) (type: boolean)

Write “@itemx” for the second line: both will show up next to one
another.

> +Whether the executables in Elogind's hook directories (see above) can

I believe “elogind” is usually spelled lower-case.

> +@item @code{broadcast-power-off-interrupts?} (default: @code{#t}) (type: boolean)
> +@item @code{broadcast-suspend-interrupts?} (default: @code{#t}) (type: boolean)

“@itemx” as well.

> +(define (/etc/elogind config)

By convention, I’d call it ‘elogind-etc-directory’.
> +     #~(let ((sleep-dir (string-append #$output "/system-sleep/"))
> +             (shutdown-dir (string-append #$output "/system-shutdown/")))
> +         (use-modules (guix build utils))

‘use-modules’ should always be used at the top level; it’s not
guaranteed to work otherwise (it wouldn’t work if the module exports
macros, for instance).

Also, please avoid abbreviations.

So that gives something like:

  #~(begin
      (use-modules (guix build utils))

      (define sleep-directory (string-append #$output "/system-sleep"))
      (define shutdown-directory (string-append #$output "/system-shutdown"))

      …)

Apart from these minor issues, the patch looks great to me.

Could you send an updated version?

Thanks!

Ludo’.