xubuntu-dev team mailing list archive

[Steve Langasek <mp+435314@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing



Diff comments:

> diff --git a/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed b/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed
> new file mode 100644
> index 0000000..cf77909
> --- /dev/null
> +++ b/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed
> @@ -0,0 +1,7 @@
> +# Enable extras.ubuntu.com.
> +d-i	apt-setup/extras	boolean true

extras.ubuntu.com is obsolete, this should be dropped.

> +# Install the Xubuntu desktop.
> +tasksel	tasksel/first	multiselect xubuntu-core

With the rename of the image to Xubuntu Minimal, should we also make this metapackage be xubuntu-minimal with xubuntu-core made a compatibility package depending on it?

> +d-i	preseed/early_command	string . /usr/share/debconf/confmodule; db_get debconf/priority; case $RET in low|medium) db_fset tasksel/first seen false; echo 'tasksel tasksel/first seen false' >>/var/lib/preseed/log ;; esac

this also looks like a bit of legacy cargo-culting, does Xubuntu ever use tasksel during the install?  This is very specific to d-i

> +# No XFCE translation packages yet.
> +d-i	pkgsel/language-pack-patterns	string
> diff --git a/tools/add_live_filesystem b/tools/add_live_filesystem
> index 952ca36..23e362d 100755
> --- a/tools/add_live_filesystem
> +++ b/tools/add_live_filesystem
> @@ -122,6 +122,8 @@ case $PROJECT in
>  				;;
>  		esac
>  		;;
> +	xubuntu-minimal)
> +		RELEASE_NOTES="http://www.ubuntu.com/getubuntu/releasenotes?os=xubuntu&ver=${DEBVERSION%% *}&lang=\${LANG}"

This is missing the ';;' required to close out the case entry in shell.  But also it duplicates a lot of text, so might I suggest instead something like this:

case $PROJECT in
        ubuntu|kubuntu|edubuntu|xubuntu|xubuntu-minimal|gobuntu|ubuntustudio|mythbuntu|lubuntu|ubuntu-gnome|ubuntukylin|ubuntu-desktop-next|ubuntu-mate|ubuntu-budgie)
                case $DIST in
                        warty|hoary|breezy|dapper|edgy)
                                ;;
                        *)
                                [ "$PROJECT" = xubuntu-minimal ] && PROJECT=xubuntu
                                RELEASE_NOTES="http://www.ubuntu.com/getubuntu/releasenotes?os=$PROJECT&ver=${DEBVERSION%% *}&lang=\${LANG}"
                                ;;
                esac
                ;;
esac

>  esac
>  if [ "$RELEASE_NOTES" ]; then
>  	mkdir -p "$BDIR/CD1/.disk"
> diff --git a/tools/boot/lunar/common.sh b/tools/boot/lunar/common.sh
> index 349f024..9746f3a 100644
> --- a/tools/boot/lunar/common.sh
> +++ b/tools/boot/lunar/common.sh
> @@ -23,6 +23,9 @@ default_kernel_params() {
>          xubuntu)
>              KERNEL_PARAMS="${KERNEL_PARAMS:+$KERNEL_PARAMS }file=/cdrom/preseed/xubuntu.seed maybe-ubiquity quiet splash --- "
>              ;;
> +        xubuntu-minimal)
> +            KERNEL_PARAMS="${KERNEL_PARAMS:+$KERNEL_PARAMS }file=/cdrom/preseed/xubuntu-minimal.seed quiet splash --- "
> +            ;;

Can I ask why xubuntu uses maybe-ubiquity, but xubuntu-minimal does not?

Do you anticipate a reason for the xubuntu-minimal.seed to ever differ from xubuntu.seed?  Perhaps instead of adding a new xubuntu-minimal.seed file to the debian-cd tree, it would make sense to make the previous entry xubuntu|xubuntu-minimal) and simply keep the boot options between the two images in sync that way?

>          ubuntustudio)
>              KERNEL_PARAMS="${KERNEL_PARAMS:+$KERNEL_PARAMS }file=/cdrom/preseed/ubuntustudio.seed maybe-ubiquity quiet splash --- "
>              ;;


-- 
https://code.launchpad.net/~xubuntu-dev/debian-cd/+git/debian-cd/+merge/435314
Your team Xubuntu Developers is subscribed to branch ~xubuntu-dev/debian-cd:xubuntu-core.