[prev in list] [next in list] [prev in thread] [next in thread]
List: gentoo-dev
Subject: Re: [gentoo-dev] RFC: intel-sdp.eclass check user license
From: justin <jlec () gentoo ! org>
Date: 2012-11-28 7:37:13
Message-ID: 50B5BF29.1080701 () gentoo ! org
[Download RAW message or body]
On 28/11/12 00:04, Mike Frysinger wrote:
> On Tuesday 27 November 2012 07:26:50 justin wrote:
>> next patch for intel-sdp.eclass
>
> your code has a lot of whitespace damage (leading spaces instead of tabs).
> you should fix that up.
I am sorry for that and we fix it up. Did some writing on mac where the
editor did magic tab -> whitespace conversion.
>
>> +# @ECLASS-FUNCTION: big-warning
>> +# @INTERNAL
>> +# warn user that we really require a license
>
> there should be @DESCRIPTION line before the description
>
I have overlooked that. Fixed now.
> you can run /usr/portage/app-portage/eclass-manpages/files/eclass-to-manpage.sh
> against the eclass to check for errors.
Didn't know, that you can run it on single files. Nice to know, Thanks.
>
> also, just because they're @INTERNAL doesn't mean short names like "big-
> warning" and "run-test" are OK. your eclass is putting funcs into global
> scope which can collide with other eclasses/ebuilds and possibly things in
> $PATH (dejagnu provides a standard program called `runtest`). best to give
> them a unique prefix like _isdp_big_warning().
You are right. I will prefix and name them correctly.
>
>> +_version_test() {
>> + local _comp _comp_full _arch _file _warn
>
> you've declared the vars all local. there's no need for the _ prefix.
>
>> + for ((i = 0; i < ${#_dirs[@]}; i++)); do
>
> for dir in "${dirs[@]}" ; do
I can't remember what was my problem, but somehow I didn't manage to
iterate properly over the array. So I looked that up and found this syntax.
But maybe something else was wrong too.
>
> that avoids indexing dirs constantly
> -mike
>
thanks for your comments mike,
Jusitn
["signature.asc" (application/pgp-signature)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic