debian-cd

Merge lp://qastaging/~mwhudson/debian-cd/document-xorriso-options into lp://qastaging/~ubuntu-cdimage/debian-cd/ubun3

  1. document-xorriso-options
  2. Merge into ubun3
Proposed by Michael Hudson-Doyle
Status: Merged
Merged at revision: 2079
Proposed branch: lp://qastaging/~mwhudson/debian-cd/document-xorriso-options
Merge into: lp://qastaging/~ubuntu-cdimage/debian-cd/ubun3
Prerequisite: lp://qastaging/~xnox/debian-cd/unbreak-grub2-hybrid-iso
Diff against target: 84 lines (+74/-1)
1 file modified
tools/boot/groovy/boot-amd64 (+74/-1)
To merge this branch: bzr merge lp://qastaging/~mwhudson/debian-cd/document-xorriso-options
Reviewer Review Type Date Requested Status
Steve Langasek Approve
Paride Legovini Approve
Review via email: mp+387015@code.qastaging.launchpad.net

Description of the change

Add a lot of words. Do not change behaviour (hopefully).

To post a comment you must log in.
Revision history for this message
Steve Langasek (vorlon) wrote :

I agree with the overall intent here, thanks.

review: Needs Information
lp://qastaging/~mwhudson/debian-cd/document-xorriso-options updated
2080. By Michael Hudson-Doyle

expand "make various things unhappy"

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
Revision history for this message
Dimitri John Ledkov (xnox) :
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

ping

Revision history for this message
Paride Legovini (paride) wrote :

One inline comment, for the rest LGTM. I wonder if we'll ever be able to discontinue some of those legacy modes one day :)

review: Needs Fixing
Revision history for this message
Paride Legovini (paride) wrote :

The only difference I can see with the original command is that the

-eltorito-alt-boot -e boot/grub/efi.img -no-emul-boot

options block and the

-append_partition 2 0xef cd-boot-images/usr/share/cd-boot-images-amd64/tree/boot/grub/efi.img

options block are swapped. I imagine you did it on purpose to add the boot options in a logical order:

- Legacy cdrom
- Legacy disk
- UEFI cdrom
- UEFI disk

I don't see a reason why the new options order shouldn't work, but I didn't test it.

Added another inline comment.

review: Needs Fixing
lp://qastaging/~mwhudson/debian-cd/document-xorriso-options updated
2081. By Michael Hudson-Doyle

address review comments

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Thanks for looking at this!!

> The only difference I can see with the original command is that the
>
> -eltorito-alt-boot -e boot/grub/efi.img -no-emul-boot
>
> options block and the
>
> -append_partition 2 0xef cd-boot-images/usr/share/cd-boot-images-
> amd64/tree/boot/grub/efi.img
>
> options block are swapped. I imagine you did it on purpose to add the boot
> options in a logical order:
>
> - Legacy cdrom
> - Legacy disk
> - UEFI cdrom
> - UEFI disk

Right.

> I don't see a reason why the new options order shouldn't work, but I didn't
> test it.

I think I tested this at the time...

> Added another inline comment.

Thanks, fixed this and the other one.

Revision history for this message
Paride Legovini (paride) wrote :

LGTM, thanks!

review: Approve
Revision history for this message
Steve Langasek (vorlon) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches