On Tuesday 09 December 2008 14:14:05 Donnie Berkholz wrote:
> 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})your grep ignores the bigger picture. this is only in the strip-unsupported- flag function. so while we should flesh it out to include CPPFLAGS/LDFLAGS, it doesnt really address the original question. -mike
signature.asc
Description: This is a digitally signed message part.
