Subject: bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

From: [email-masked] (Ludovic Courtès)
Date: Tue, 18 Apr 2017 10:30:53 +0200

Hi Danny,

Another quick preliminary review…

Could you explain the “big picture”, the rationale behind this change?

Danny Milosavljevic <[email-masked]> skribis:

> +(define (bootable-kernel-arguments kernel-arguments system root-device)
> +  (cons* (string-append "--root=" root-device)
> +         #~(string-append "--system=" #$system)
> +         #~(string-append "--load=" #$system "/boot")
> +         kernel-arguments))

Please add a docstring to top-level procedures:

  https://www.gnu.org/software/guix/manual/html_node/Formatting-Code.html

> +(define (operating-system-all-kernel-arguments os system root-device)
> +  (bootable-kernel-arguments (operating-system-kernel-arguments os)
> +                             system
> +                             root-device))

For services, we have:

  operating-system-user-services
  operating-system-services

So I suggest stick to this convention and thus have:

  operating-system-user-kernel-arguments     ;arguments specified by the user
  operating-system-kernel-arguments          ;all the arguments

> -(define (operating-system-parameters-file os)
> +(define (operating-system-boot-parameters os system root-device)

I think it would be clearer to rename in a separate patch.

> +(define (read-boot-parameters-file sysgen)
> +  "Read boot parameters from SYSGEN's (system or generation) \"parameters\"

Please use full words in variable names, typically ‘system’ here.

Thanks,
Ludo’.