Re: [gentoo-dev] RFC: kde5 and kde5-functions eclass

[Davide Pesavento]

kde5-functions.eclass

> inherit versionator

versionator doesn't export any phase functions so it can stay inside the
_KDE5_FUNCTIONS_ECLASS conditional block.

> case ${EAPI:-0} in

I believe :-0 is unnecessary here, "*)" will match anyway, but it doesn't
hurt either.

> *) die "EAPI=${EAPI} is not supported" ;;

Here ${EAPI:-0} is needed instead.

> if [[ ${CATEGORY} = kde-base ]]; then
> debug-print "${ECLASS}: KDEBASE ebuild recognized"
> KDEBASE=kde-base
> elif [[ ${CATEGORY} = kde-frameworks ]]; then
> debug-print "${ECLASS}: KDEFRAMEWORKS ebuild recognized"
> KDEBASE=kde-frameworks
> elif [[ ${KMNAME-${PN}} = kdevelop ]]; then
> debug-print "${ECLASS}: KDEVELOP ebuild recognized"
> KDEBASE=kdevelop
> fi

Can you move the print after the if-fi block to avoid duplication?
debug-print "${ECLASS}: ${KDEBASE} ebuild recognized"

> case ${KDE_SCM} in
> svn|git) ;;

I thought ":" was necessary for syntax reasons but apparently it's not. You
may want to add it anyway for consistency with the previous case statement.

> if [[ -a "CMakeLists.txt" ]]; then

Unnecessary quoting. Also, -e is more common than -a

>        sed -e
"/add_subdirectory[[:space:]]*([[:space:]]*${1}[[:space:]]*)/s/^/#DONOTCOMPILE
/" \

What if ${1} contains slashes?

> [[ -z ${1} ]] && die "Missing parameter"

Sanity checking of arguments should be done at the beginning of the
function body. Also, is $2 allowed to be empty?

> # @FUNCTION: add_frameworks_dep

Add @USAGE eclass doc.

> local ver=${1:-${PV}}
> local major=$(get_major_version ${ver})
> local minor=$(get_version_component_range 2 ${ver})
> local micro=$(get_version_component_range 3 ${ver})
> if [[ ${ver} == 9999 ]]; then

No wildcard match here? (*9999)

Thanks,
Davide