GNU bug report logs - #74819
[PATCH] services: elogind: Support Hook Directories

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Simon Tournier <zimon.toutoune <at> gmail.com>, paren <at> disroot.org, Tobias Geerinckx-Rice <me <at> tobias.gr>, jgart <jgart <at> dismail.de>, 74819 <at> debbugs.gnu.org, Mathieu Othacehe <othacehe <at> gnu.org>, Christopher Baines <guix <at> cbaines.net>
Subject: [bug#74819] [PATCH v3] services: elogind: Support Hook Directories
Date: Mon, 23 Dec 2024 19:00:24 +0100

Hi,

45mg <45mg.writes <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.