Review: Needs Fixing

Let me know if you want to rebase and merge this to deal with the unicode and 
whitespace breakage on my side.  Want to get agreement on the dropping of the 
xubuntu-minimal-specific preseed stuff first tho!

Diff comments:

> diff --git a/Makefile b/Makefile
> index c0440fc..c5d0d03 100755
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@
>  
>  # Main Makefile for YACS
>  #
> -# Copyright 1999 Rapha�l Hertzog <hert...@debian.org>
> +# Copyright 1999 Rapha�l Hertzog <hert...@debian.org>

Unfortunately this file is in iso8859-1 encoding.  Either this file can be 
converted to utf-8, or this line needs to be preserved. (in the diff it appears 
the non-utf8 character was replaced by a literal '�'

>  # See the README file for the license
>  
>  # The environment variables must have been set
> @@ -251,34 +251,42 @@ else
>   CDBASE = $(CODENAME)-desktop-$(FULLARCH)
>      endif
>     else
> -    ifneq (,$(findstring $(CODENAME),warty hoary breezy))
> - CDBASE = $(CODENAME)-live-$(FULLARCH)
> +     ifeq ($(PROJECT),xubuntu)

space-to-tab munging; please use 4 spaces instead of a tab

> +     ifeq ($(SUBPROJECT),minimal)
> + CDBASE = $(CODENAME)-minimal-$(FULLARCH)
> +     else
> + CDBASE = $(CODENAME)-desktop-$(FULLARCH)
> +     endif
>      else
> -     ifeq ($(PROJECT),ubuntu-server)
> - CDBASE = $(CODENAME)-live-server-$(FULLARCH)
> +     ifneq (,$(findstring $(CODENAME),warty hoary breezy))
> + CDBASE = $(CODENAME)-live-$(FULLARCH)

inserting it at the top makes for a confusing diff.  Could we add the minimal 
subproject handling toward the bottom instead, where the existing SUBPROJECT 
cases are?  I don't think it needs to be guarded by a xubuntu check, if someone 
else wanted a 'minimal' image I think we would want to default to matching 
handling.

>       else
> -      ifeq ($(PROJECT),ubuntu-mid)
> - CDBASE = $(CODENAME)-mid-$(FULLARCH)
> +      ifeq ($(PROJECT),ubuntu-server)
> + CDBASE = $(CODENAME)-live-server-$(FULLARCH)
>        else
> -       ifeq ($(PROJECT),ubuntu-netbook)
> - CDBASE = $(CODENAME)-netbook-$(FULLARCH)
> +       ifeq ($(PROJECT),ubuntu-mid)
> + CDBASE = $(CODENAME)-mid-$(FULLARCH)
>         else
> -        ifeq ($(PROJECT),kubuntu-netbook)
> +        ifeq ($(PROJECT),ubuntu-netbook)
>   CDBASE = $(CODENAME)-netbook-$(FULLARCH)
>          else
> -         ifeq ($(PROJECT),ubuntu-moblin-remix)
> - CDBASE = $(CODENAME)-moblin-remix-$(FULLARCH)
> +         ifeq ($(PROJECT),kubuntu-netbook)
> + CDBASE = $(CODENAME)-netbook-$(FULLARCH)
>           else
> -          ifeq ($(PROJECT),kubuntu-mobile)
> - CDBASE = $(CODENAME)-mobile-$(FULLARCH)
> +          ifeq ($(PROJECT),ubuntu-moblin-remix)
> + CDBASE = $(CODENAME)-moblin-remix-$(FULLARCH)
>            else
> -           ifeq ($(SUBPROJECT),canary)
> - CDBASE = $(CODENAME)-desktop-canary-$(FULLARCH)
> +           ifeq ($(PROJECT),kubuntu-mobile)
> + CDBASE = $(CODENAME)-mobile-$(FULLARCH)
>             else
> -            ifeq ($(SUBPROJECT),legacy)
> - CDBASE = $(CODENAME)-desktop-legacy-$(FULLARCH)
> +            ifeq ($(SUBPROJECT),canary)
> + CDBASE = $(CODENAME)-desktop-canary-$(FULLARCH)
>              else
> +             ifeq ($(SUBPROJECT),legacy)
> + CDBASE = $(CODENAME)-desktop-legacy-$(FULLARCH)
> +             else
>   CDBASE = $(CODENAME)-desktop-$(FULLARCH)
> +             endif
>              endif
>             endif
>            endif
> @@ -287,7 +295,7 @@ else
>         endif
>        endif
>       endif
> -    endif
> +     endif

whitespace

>     endif
>    endif
>   endif
> @@ -1285,7 +1293,7 @@ pi-makelist:
>  # Generate only one image number $(CD)
>  image: bin-image
>  bin-image: ok bin-md5list $(OUT)
> -     @echo "Generating the binary iso image n�$(CD) ..."
> +     @echo "Generating the binary iso image n�$(CD) ..."
>       @test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)

more unicode breakage

>       set -e; cd $(BDIR); opts=`cat $(CD).mkisofs_opts`; \
>        volid=`cat $(CD).volid`; \
> @@ -1309,7 +1317,7 @@ bin-image: ok bin-md5list $(OUT)
>           fi
>  
>  src-image: ok src-md5list $(OUT)
> -     @echo "Generating the source iso image n�$(CD) ..."
> +     @echo "Generating the source iso image n�$(CD) ..."

more unicode breakage

>       @test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)
>       set -e; cd $(SDIR); opts=`cat $(CD).mkisofs_opts`; \
>        volid=`cat $(CD).volid`; \
> 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..8751c89
> --- /dev/null
> +++ b/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed
> @@ -0,0 +1,5 @@
> +# Install the Xubuntu desktop.
> +tasksel      tasksel/first   multiselect xubuntu-desktop-minimal
> +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
> +# No XFCE translation packages yet.
> +d-i  pkgsel/language-pack-patterns   string

I still think this can be dropped and use a single xubuntu.seed for both 
images.  ubiquity doesn't make any use of tasksel for determining what packages 
to install on the target system - it simply installs the packages already 
present as part of the live image!



-- 
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.


_______________________________________________
Mailing list: https://launchpad.net/~xubuntu-dev
Post to     : xubuntu-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~xubuntu-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to