> > It's LGTM for implementation part, but I think the unit test for it is
> > required.
>
> There seems to be unit tests already for the `update_grub_file` function [1].
> Would I need to add something additionally to test for the change made in the
> template file?
>
> [1]: https://git.launchpad.net/charm-
> sysconfig/tree/src/tests/unit/test_lib.py#n250
>
> > Also please provide more detail to explain why this change works.
>
> ```
> -{% else -%}
> -GRUB_CMDLINE_LINUX_DEFAULT=""
> ```
>
> This is to allow inheriting the pre-existing kernel cmdline parameters the
> default option. With the charm no longer resetting the value of
> `GRUB_CMDLINE_LINUX_DEFAULT` to an empty string, it'll only append to the
> already existing values (ex: coming from MAAS).
>
> ```
> -GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT console=ttyS0,115200
> console=tty0"
> +GRUB_CMDLINE_LINUX_DEFAULT="console=ttyS0,115200 console=tty0
> $GRUB_CMDLINE_LINUX_DEFAULT"
> ```
>
> These parameters have been switched around so that the pre-existing console
> values (ex: coming from MAAS) will not be overwritten by the charm. According
> to my understanding, the console parameters (if defined) from
> `$GRUB_CMDLINE_LINUX_DEFAULT` will take precedence since they are declared the
> last.
This is a little tricky to me. There already a precedence design under /etc/grub.d/ folder.
If we want to put "console=ttyS0,115200 console=tty0" as a "nice to have" option, we should put it into 00xxxx. But now, we put it into 90sysconfig (almost the biggest number) and use this way to avoid taking precedence.
I don't know if other people have the same feeling. I suggest that we should find the stakeholders to discuss which one should have higher precedence and solve it in a correct way.
> > It's LGTM for implementation part, but I think the unit test for it is /git.launchpad. net/charm- tree/src/ tests/unit/ test_lib. py#n250 LINUX_DEFAULT= "" LINUX_DEFAULT` to an empty string, it'll only append to the LINUX_DEFAULT= "$GRUB_ CMDLINE_ LINUX_DEFAULT console= ttyS0,115200 LINUX_DEFAULT= "console= ttyS0,115200 console=tty0 LINUX_DEFAULT" CMDLINE_ LINUX_DEFAULT` will take precedence since they are declared the
> > required.
>
> There seems to be unit tests already for the `update_grub_file` function [1].
> Would I need to add something additionally to test for the change made in the
> template file?
>
> [1]: https:/
> sysconfig/
>
> > Also please provide more detail to explain why this change works.
>
> ```
> -{% else -%}
> -GRUB_CMDLINE_
> ```
>
> This is to allow inheriting the pre-existing kernel cmdline parameters the
> default option. With the charm no longer resetting the value of
> `GRUB_CMDLINE_
> already existing values (ex: coming from MAAS).
>
> ```
> -GRUB_CMDLINE_
> console=tty0"
> +GRUB_CMDLINE_
> $GRUB_CMDLINE_
> ```
>
> These parameters have been switched around so that the pre-existing console
> values (ex: coming from MAAS) will not be overwritten by the charm. According
> to my understanding, the console parameters (if defined) from
> `$GRUB_
> last.
This is a little tricky to me. There already a precedence design under /etc/grub.d/ folder. ttyS0,115200 console=tty0" as a "nice to have" option, we should put it into 00xxxx. But now, we put it into 90sysconfig (almost the biggest number) and use this way to avoid taking precedence.
If we want to put "console=
I don't know if other people have the same feeling. I suggest that we should find the stakeholders to discuss which one should have higher precedence and solve it in a correct way.