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