Re: [gentoo-dev] Announcement of The G Palmtop Environment ebuilds

[Petteri Räty]

Angelo Arrifano wrote:
>
> # Copyright 2008 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: $
> #
> # Authors:
> # Rene Wagner <rw@×××××××××.org>
> # Ned Ludd <solar@g.o>
> # Angelo Arrifano <miknix@g.o>
>

Should use eclass-manpages syntax.

>
> # GPE ECLASS
> #GPECONF=""             # extra configure opts passed to econf
> ELTCONF=""              # extra options passed to elibtoolize
> DOCS=""                 # documentation files to be installed with dodoc
>

If other eclass that comes before in the inherit hierarchy and sets
DOCS, do we want to override it?

> [ -z "${GPE_MIRROR}" ] && export GPE_MIRROR="http://gpe.linuxtogo.org/download/source"
>
> [ -z "${GPE_TARBALL_SUFFIX}" ] && export GPE_TARBALL_SUFFIX="gz"
>

Is there a binary called that makes use of those two?

>
> IUSE="${IUSE} nls"
>

This is the first use of IUSE in the eclass so there is nothing to
append to.

>
> gpe_src_configure() {
> 	tc-export CC
> 	if [ -f configure ]; then
> 		elibtoolize ${ELTCONF}
> 		econf "$@" ${GPECONF} || die "./configure failure"
> 	fi
> }
>

Ebuilds/Eclasses should use [[ instead of [ and econf dies on it's own
any way.


> gpe_src_install() {
> 	USE_NLS=yes
> 	use nls || USE_NLS=no
>

I don't see USE_NLS used outside install so it should be local and
written in lower case.

> 	if [ -f configure ]; then
> 		einstall "$@"
> 	else

If you really need to use einstall, it would be best to add a comment
about why it's needed.

> 		make DESTDIR=${D} PREFIX=/usr \
> 			STRIP=true ENABLE_NLS=${USE_NLS} \
> 			"$@" install
> 	fi
>

Should use emake. Stripping should be left to the package manager.

> 	# manual document installation
> 	[ -n "${DOCS}" ] && dodoc ${DOCS}
>
> }
>

dodoc should have || die with it

>
> EXPORT_FUNCTIONS src_compile src_install src_unpack
>

Never exports configure for EAPI 2.

Regards,
Petteri