On 10:33 Mon 08 Dec     , Jeremy Olexa wrote:
> Hello,
> I am seeking a positive code review on the following change to
> flag-o-matic.eclass, diff is below (reasons are below that):
> 
> %% cvs diff
> Index: flag-o-matic.eclass
> ===================================================================
> RCS file: /var/cvsroot/gentoo-x86/eclass/flag-o-matic.eclass,v
> retrieving revision 1.126
> diff -u -r1.126 flag-o-matic.eclass
> --- flag-o-matic.eclass 3 Nov 2008 05:52:39 -0000       1.126
> +++ flag-o-matic.eclass 25 Nov 2008 18:36:04 -0000
> @@ -417,7 +417,8 @@
> 
>        x=""
>        for x in "$@" ; do
> -               test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}"
> +               test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" || \
> +                       ewarn "removing ${x} because ${comp} rejected it"
>        done
> 
>        echo "${flags}"
> @@ -656,7 +657,7 @@
>                        ewarn "Appending a library link instruction
> (${flag}); libraries to link to should not be passed through LDFLAGS"
>        done
> 
> -       export LDFLAGS="${LDFLAGS} $*"
> +       export LDFLAGS="${LDFLAGS} $(test-flags "$@")"
>        return 0
>  }

I like the consistency with other flags:

comet $ grep FLAGS.*test-fl /usr/portage/eclass/flag-o-matic.eclass 
	export CFLAGS=$(test-flags-CC ${CFLAGS})
	export CXXFLAGS=$(test-flags-CXX ${CXXFLAGS})
	export FFLAGS=$(test-flags-F77 ${FFLAGS})
	export FCFLAGS=$(test-flags-FC ${FCFLAGS})

-- 
Thanks,
Donnie

Donnie Berkholz
Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com