Subject: bug#26339: [PATCH 07/18] system: Add bootloader type.

From: Danny Milosavljevic <[email-masked]>
Date: Sat, 15 Apr 2017 18:26:34 +0200

LGTM mostly.  <Insert same comment about the same names as in the other patches>

> -(define* (grub-configuration #:optional (config (bootloader-configuration)))
> -  (bootloader-configuration
> -   (inherit config)
> -   (bootloader (@ (gnu packages bootloaders) grub))

Hmm.  Does that mean the user can't replace the bootloader package in the configuration?

> -(define* (grub-efi-configuration #:optional (config (bootloader-configuration)))
> -  (bootloader-configuration
> -   (inherit (grub-configuration config))
> -   (bootloader (@ (gnu packages bootloaders) grub-efi))))

Oh, so that's how.  I think it would be nice-to-have if the user could also override the bootloader package used.

> -   (install-procedure install-syslinux)))

Wait, so install-syslinux wouldn't need to be exported from this module at all, right?

Still nice that the user can use it - though is it worth exposing the installers?

Otherwise LGTM!