Jory A. Pratt wrote: > Unless there are any major objections tomorrow night I will commit > mozextension.eclass to the tree. You can find mozextension.eclass at > http://dev.gentoo.org/~anarchy/eclass . You can also find the firefox > and firefox-bin ebuilds at http://dev.gentoo.org/~anarchy/ebuilds that > are waiting to go to be added to the tree which inherit mozextension. >
http://dev.gentoo.org/~anarchy/eclass/mozconfig-2.eclass if use debug; then mozconfig_annotate +debug \ --enable-debug \ --enable-tests \ --disable-reorder \ --disable-strip \ --disable-strip-libs \ --enable-debugger-info-modules=ALL_MODULES else mozconfig_annotate -debug \ --disable-debug \ --disable-tests \ --enable-reorder \ --enable-strip \ --enable-strip-libs I would use a local variable for the common options to avoid code duplication. http://dev.gentoo.org/~anarchy/eclass/mozcoreconf.eclass declare MOZ=$([[ ${PN} == mozilla || ${PN} == gecko-sdk ]] && echo true || echo false) declare FF=$([[ ${PN} == *firefox ]] && echo true || echo false) declare TB=$([[ ${PN} == *thunderbird ]] && echo true || echo false) declare SB=$([[ ${PN} == *sunbird ]] && echo true || echo false) declare EM=$([[${PN} == *enig]] && echo true || echo false) [[${PN} == *enig]] && echo true || echo false This seems to be duplicated here many times. Consider making this a helper function. mozconfig_final() { declare ac opt hash reason You seem to be using declare to keep variables local. I think it is more common to use the local keyword for this: [EMAIL PROTECTED] /usr/portage/eclass $ grep local * | wc -l 810 [EMAIL PROTECTED] /usr/portage/eclass $ grep declare * | wc -l 41 Why is mozconfig_final here three times? http://dev.gentoo.org/~anarchy/eclass/mozextension.eclass Consider adding debug-print calls to your functions. if [ "${x:0:2}" = "./" ] ; then srcdir="" else srcdir="${DISTDIR}/" fi srcdir is not declared local in the beginning of the function Check this for other variables too. cd "${WORKDIR}" || die "$myfail" for consistency you should probably use ${myfail} [ ${#} -ne 1 ] && die "'xpi_install' takes exactly one argument" you can use FUNCNAME variable to get the name of the function cd ${x} Don't know if this can contain spaces but best to be sure and add quotes. I also noticed a couple of other places where pathnames are not quoted. emid=$(sed -n '/<\?em:id>\?/!d; s/.*\([\"{].*[}\"]\).*/\1/; s/\"//g; p; q' ${x}/install.rdf) Consider adding some comments about what this is supposed to do so that people who are looking at the code the first time get some kind of an idea. Regards, Petteri
signature.asc
Description: OpenPGP digital signature
