1 |
On 10:33 Mon 08 Dec , Jeremy Olexa wrote: |
2 |
> Hello, |
3 |
> I am seeking a positive code review on the following change to |
4 |
> flag-o-matic.eclass, diff is below (reasons are below that): |
5 |
> |
6 |
> %% cvs diff |
7 |
> Index: flag-o-matic.eclass |
8 |
> =================================================================== |
9 |
> RCS file: /var/cvsroot/gentoo-x86/eclass/flag-o-matic.eclass,v |
10 |
> retrieving revision 1.126 |
11 |
> diff -u -r1.126 flag-o-matic.eclass |
12 |
> --- flag-o-matic.eclass 3 Nov 2008 05:52:39 -0000 1.126 |
13 |
> +++ flag-o-matic.eclass 25 Nov 2008 18:36:04 -0000 |
14 |
> @@ -417,7 +417,8 @@ |
15 |
> |
16 |
> x="" |
17 |
> for x in "$@" ; do |
18 |
> - test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" |
19 |
> + test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" || \ |
20 |
> + ewarn "removing ${x} because ${comp} rejected it" |
21 |
> done |
22 |
> |
23 |
> echo "${flags}" |
24 |
> @@ -656,7 +657,7 @@ |
25 |
> ewarn "Appending a library link instruction |
26 |
> (${flag}); libraries to link to should not be passed through LDFLAGS" |
27 |
> done |
28 |
> |
29 |
> - export LDFLAGS="${LDFLAGS} $*" |
30 |
> + export LDFLAGS="${LDFLAGS} $(test-flags "$@")" |
31 |
> return 0 |
32 |
> } |
33 |
|
34 |
I like the consistency with other flags: |
35 |
|
36 |
comet $ grep FLAGS.*test-fl /usr/portage/eclass/flag-o-matic.eclass |
37 |
export CFLAGS=$(test-flags-CC ${CFLAGS}) |
38 |
export CXXFLAGS=$(test-flags-CXX ${CXXFLAGS}) |
39 |
export FFLAGS=$(test-flags-F77 ${FFLAGS}) |
40 |
export FCFLAGS=$(test-flags-FC ${FCFLAGS}) |
41 |
|
42 |
-- |
43 |
Thanks, |
44 |
Donnie |
45 |
|
46 |
Donnie Berkholz |
47 |
Developer, Gentoo Linux |
48 |
Blog: http://dberkholz.wordpress.com |