Code review comment for ~alkisg/ubuntu/+source/ipxe:lp1811496

Revision history for this message
Bryce Harrington (bryce) wrote :

Running shellcheck on 20_ipxe would likely suggest some stylistic improvements such as quoting and bracketing of variables. I think that's good practice, but not critical.

I'm not keen on splitting the heredoc with the `save_default_entry | grub_add_tab` call in the middle, but there's nothing technically wrong with it. I might try doing something like:

   echo "Found iPXE image: $IPXE" >&2
   entry=$(save_default_entry | grub_add_tab)
   cat << EOF
menuentry "Network boot (iPXE)" --users "" --class network {
${prepare_boot_cache}
${entry}
 if [ "\$grub_platform" = "efi" ]; then
  chainloader $IPXEPATH.efi
 else
  linux16 $IPXEPATH.lkrn
  # If the user provided an iPXE script, load it
  if [ -f $IPXEPATH.ipxe ]; then
   initrd16 $IPXEPATH.ipxe
  fi
 fi
 }
 EOF
 fi

Unfortunately I don't know enough about ipxe to provide meaningful feedback on the grub code, but presuming it works in testing, the syntax looks ok.

review: Approve

« Back to merge proposal