1 |
On 28/11/12 00:04, Mike Frysinger wrote: |
2 |
> On Tuesday 27 November 2012 07:26:50 justin wrote: |
3 |
>> next patch for intel-sdp.eclass |
4 |
> |
5 |
> your code has a lot of whitespace damage (leading spaces instead of tabs). |
6 |
> you should fix that up. |
7 |
|
8 |
I am sorry for that and we fix it up. Did some writing on mac where the |
9 |
editor did magic tab -> whitespace conversion. |
10 |
|
11 |
> |
12 |
>> +# @ECLASS-FUNCTION: big-warning |
13 |
>> +# @INTERNAL |
14 |
>> +# warn user that we really require a license |
15 |
> |
16 |
> there should be @DESCRIPTION line before the description |
17 |
> |
18 |
|
19 |
I have overlooked that. Fixed now. |
20 |
|
21 |
> you can run /usr/portage/app-portage/eclass-manpages/files/eclass-to-manpage.sh |
22 |
> against the eclass to check for errors. |
23 |
|
24 |
Didn't know, that you can run it on single files. Nice to know, Thanks. |
25 |
|
26 |
> |
27 |
> also, just because they're @INTERNAL doesn't mean short names like "big- |
28 |
> warning" and "run-test" are OK. your eclass is putting funcs into global |
29 |
> scope which can collide with other eclasses/ebuilds and possibly things in |
30 |
> $PATH (dejagnu provides a standard program called `runtest`). best to give |
31 |
> them a unique prefix like _isdp_big_warning(). |
32 |
|
33 |
You are right. I will prefix and name them correctly. |
34 |
|
35 |
> |
36 |
>> +_version_test() { |
37 |
>> + local _comp _comp_full _arch _file _warn |
38 |
> |
39 |
> you've declared the vars all local. there's no need for the _ prefix. |
40 |
> |
41 |
>> + for ((i = 0; i < ${#_dirs[@]}; i++)); do |
42 |
> |
43 |
> for dir in "${dirs[@]}" ; do |
44 |
|
45 |
I can't remember what was my problem, but somehow I didn't manage to |
46 |
iterate properly over the array. So I looked that up and found this syntax. |
47 |
But maybe something else was wrong too. |
48 |
|
49 |
|
50 |
> |
51 |
> that avoids indexing dirs constantly |
52 |
> -mike |
53 |
> |
54 |
|
55 |
thanks for your comments mike, |
56 |
|
57 |
Jusitn |