On 12/12/2022 03:15, Cyril Brulebois wrote:
> I've glanced at depthcharge-tools-installer and while I'm no debconf
> expert and I can't assess the postinst with high certainty, the overall
> impression was good enough for me to sign and upload; I did change the
> Maintainer to debian-boot@ (same as other packages under installer-team/)
> and moved you to Uploaders, so you might want to subscribe via tracker
> once the package is accepted if you don't follow debian-boot@.

Thanks for the upload, and for the tracker subscription tip. I filter
debian-boot@ into a folder that I do try to read, but a way that could
notify me more prominently could be better.

> [...]
> A couple of nitpicks/questions anyway:
>  - depthcharge_tools_set_board():
>    The generated comment mentions preseed while I don't think preseed is
>    involved at all, and I suppose the comment should just mention
>    debian-installer instead?

It's only created if depthcharge-tools-installer/board is set. That
template is not asked as a user-visible question, but can be set via a
preseed file or in the kernel command line [1]. I do the latter for
testing things in a VM, and meant that by 'preseed'.

[1] Using boot parameters to preseed questions

>  - initramfs_tools_conf():
>    Having MODULES overriden in a separate config file might be surprising
>    to admins. Did you consider adjusting this variable directly in the
>    main initramfs-tools config file instead?

I tried to match what base-installer does [2]. In general, I think it's
better to prefer config.d mechanisms over modifying the config files, to
avoid conffile conflicts if/when the defaults change later on.

[2] base-installer initramfs-tools driver inclusion policy handling

>  - isinstallable:
>    It mentions “these values” but only checks for one. Should there other
>    patterns in that grep?

Outdated comment that I missed. There's also cros_efi and cros_legacy,
but they are included in some (apparently broken) GRUB/syslinux .cfg
files to tell ChromeOS that we're not using its verified boot method. I
used to check for them until I figured out it doesn't make sense to.

Reply via email to