Review: Needs Fixing

Thanks, much closer to what I was suggesting.  A couple more comments inline.

Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index 1f8e950..2c01e10 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -880,15 +880,45 @@ case $PROJECT in
>               ;;
>  
>       xubuntu)
> +             # Xubuntu now ships the new installer.
> +             touch config/universe-enabled
> +             PASSES_TO_LAYERS="true"
> +             KERNEL_FLAVOURS=generic
> +             # the minimal layer, for minimal installs (minimal and desktop 
> iso)
> +             add_task minimal minimal standard xubuntu-minimal
> +             cat <<-EOF > config/minimal.catalog-in.yaml
> +                     name: "Xubuntu Minimal"
> +                     description: >-
> +                     A minimal installation of the Xubuntu Desktop.
> +                     id: xubuntu-minimal
> +                     type: fsimage-layered
> +                     variant: desktop
> +                     locale_support: langpack
> +             EOF
>               case ${SUBPROJECT:-} in
>                       minimal)
> -                             add_task install minimal standard 
> xubuntu-minimal
> -                             ;;
> +                             echo "default: yes" >> 
> config/minimal.catalog-in.yaml

I *suspect* that when there's only one catalog entry, the 'default: yes' is 
unnecessary.  So you shouldn't need to set 'default: yes' for minimal: it's 
always either the *only* entry, or it's the non-default one?

If it *is* required, then you have a yaml indentation issue here anyway; you 
need the whitespace in the output to align with the block above.

> +                             add_task minimal.live xubuntu-minimal-live
> +                             add_package minimal.live linux-$KERNEL_FLAVOURS

Aside from a bug here (there is no "xubuntu-minimal-live" task, only 
"xubuntu-live", these two lines differ from the lines below only in the name of 
the parent layer of the live layer.  So what I would suggest is to do this here:
    LIVE_LAYER_PARENT=minimal
and below do:
    LIVE_LAYER_PARENT=minimal.standard

and then in the next bit down do
    add_task ${LIVE_LAYER_PARENT}.live xubuntu-live
    add_package ${LIVE_LAYER_PARENT}.live linux-$KERNEL_FLAVORS

it doesn't reduce the number of lines of code but it does reduce code 
duplication which is a plus.

>                       *)
> -                             add_task install minimal standard 
> xubuntu-desktop
> +                             # the standard layer, for standard installs 
> (desktop iso)
> +                             add_task minimal.standard xubuntu-desktop
> +                             cat <<-EOF > 
> config/minimal.standard.catalog-in.yaml
> +                                     name: "Xubuntu Desktop"
> +                                     description: >-
> +                                     A full featured Xubuntu Desktop.
> +                                     id: xubuntu-desktop
> +                                     type: fsimage-layered
> +                                     variant: desktop
> +                                     locale_support: langpack
> +                                     default: yes
> +                             EOF
> +                             add_task minimal.standard.live xubuntu-live
> +                             add_package minimal.standard.live 
> linux-$KERNEL_FLAVOURS
>                               ;;
>               esac
> -             LIVE_TASK='xubuntu-live'
> +             /usr/share/livecd-rootfs/checkout-translations-branch \
> +                     https://git.launchpad.net/subiquity po 
> config/catalog-translations
>               ;;
>  
>       ubuntu-budgie)


-- 
https://code.launchpad.net/~xubuntu-dev/livecd-rootfs/+git/livecd-rootfs/+merge/460722
Your team Xubuntu Developers is subscribed to branch 
~xubuntu-dev/livecd-rootfs:xubuntu-new-installer.


_______________________________________________
Mailing list: https://launchpad.net/~xubuntu-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~xubuntu-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to