xubuntu-dev team mailing list archive

[Steve Langasek <mp+460722@xxxxxxxxxxxxxxxxxx>]

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.