Re: [docs] [PATCH] ref-manual: Describe grub-efi-cfg overrides and GRUB_TITLE

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi Simon,

On Wed Jan 15, 2025 at 9:07 AM CET, Simon A. Eugster via lists.yoctoproject.org wrote:
> This patch describes how to use overrides for grub-efi-cfg because that
> is otherwise only clear after reading the .bbclass file. It also adds
> a description for GRUB_TITLE.
> ---
>  documentation/ref-manual/classes.rst | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
> index 761be7b545..460f94b232 100644
> --- a/documentation/ref-manual/classes.rst
> +++ b/documentation/ref-manual/classes.rst
> @@ -987,10 +987,14 @@ introspection. This functionality is only enabled if the
>  The :ref:`ref-classes-grub-efi` class provides ``grub-efi``-specific functions for
>  building bootable images.
>  
> -This class supports several variables:
> +This class supports several variables.
> +The label specific override names are ``grub_LABEL``.

"...are defined as ``grub_LABEL``"?

> +To specificy an override for the label ``mylabel``, the override name is ``grub_mylabel``,

s/specificy/specify/

Also please wrap the lines to 80 chars, as recommended by our standards.md file.

> +for example ``APPEND:grub_mylabel = my-appends``.

You are missing quotes in this example, it should be ``APPEND:grub_mylabel = "my-appends"``.

I think it would also make sense to give this example after the bullet list. I
would move the text starting from "The label specific override names..." to the
APPEND example, after the bullet list. And you keep your real life example with
the factory.

>
>  -  :term:`INITRD`: Indicates list of filesystem images to
>     concatenate and use as an initial RAM disk (initrd) (optional).
> +   Can be specified for each ``LABEL``.
>
>  -  :term:`ROOTFS`: Indicates a filesystem image to include
>     as the root filesystem (optional).
> @@ -1004,6 +1008,9 @@ This class supports several variables:
>  -  :term:`APPEND`: An override list of append strings for
>     each ``LABEL``.
>
> +-  :term:`GRUB_TITLE`: A custom title for each ```LABEL``.

In addition to adding the variable here, you also need to add its definition to
documentation/ref-manual/variables.rst, otherwise you cannot make a
cross-reference to it here. Mind that the variables.rst file had the variables
alphabetically ordered.

> +   Defaults to ``LABEL`` if it is not defined.
> +
>  -  :term:`GRUB_OPTS`: Additional options to add to the
>     configuration (optional). Options are delimited using semi-colon
>     characters (``;``).
> @@ -1011,6 +1018,14 @@ This class supports several variables:
>  -  :term:`GRUB_TIMEOUT`: Timeout before executing
>     the default ``LABEL`` (optional).
>
> +For the label ``factory``, a custom GRUB menu entry titled "Factory Install" with the additional parameter ``factory=yes``
> +can be achieved as follows::
> +
> +  LABELS:append = "factory"

I think you need a space after the quote here:

  LABELS:append = " factory"

> +  APPEND:grub_factory = "factory=yes"
> +  GRUB_TITLE:grub_factory = "Factory Install"
> +
> +

Unneeded extra newline here

>  .. _ref-classes-gsettings:
>
>  ``gsettings``

Thank you,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com