| 1 |
please post it inline to make review easier |
| 2 |
|
| 3 |
> # @MAINTAINER: mozilla@g.o |
| 4 |
> # @AUTHOR: Nirbheek Chauhan <nirbheek@g.o> |
| 5 |
|
| 6 |
goes on newline, not inlined |
| 7 |
|
| 8 |
> # @DESCRIPTION: Array containing the list of language pack xpis available |
| 9 |
|
| 10 |
text starts on the next line, not the existing line |
| 11 |
|
| 12 |
> # @ECLASS-VARIABLE: LANGS |
| 13 |
> # @ECLASS-VARIABLE: LANGPACK_PREFIX |
| 14 |
> # @ECLASS-VARIABLE: LANGPACK_SUFFIX |
| 15 |
|
| 16 |
these prob could use MOZ prefixes as well |
| 17 |
|
| 18 |
> : ${LANGS:=""} |
| 19 |
|
| 20 |
you say it's an array but then you initialize it to a string ... |
| 21 |
|
| 22 |
> if ! [[ ${PV} =~ alpha|beta ]]; then |
| 23 |
> for x in "${LANGS[@]}" ; do |
| 24 |
|
| 25 |
x is a global variable here ... one reason to write this as an internal func |
| 26 |
and then call it so you can use `local` |
| 27 |
|
| 28 |
> if [[ ${x} = en ]] || [[ ${x} = en-US ]]; then |
| 29 |
|
| 30 |
should be == imo |
| 31 |
|
| 32 |
> SRC_URI="${SRC_URI} |
| 33 |
|
| 34 |
SRC_URI+="... |
| 35 |
|
| 36 |
> IUSE="${IUSE} linguas_${x/-/_}" |
| 37 |
|
| 38 |
IUSE+="... |
| 39 |
|
| 40 |
> mozlinguas() { |
| 41 |
|
| 42 |
missing eclass documentation |
| 43 |
|
| 44 |
> # Generate the list of language packs called "linguas" |
| 45 |
> # This list is used to unpack and install the xpi language packs |
| 46 |
|
| 47 |
shouldn't this initialize linguas=() ? |
| 48 |
|
| 49 |
and shouldn't it name the return value mozlinguas ? |
| 50 |
|
| 51 |
> # If this language is supported by ${P}, |
| 52 |
> elif has ${lingua} "${LANGS[@]//-/_}"; then |
| 53 |
> # Add the language to linguas, if it isn't already there |
| 54 |
> has ${lingua//_/-} "${linguas[@]}" || linguas+=(${lingua//_/-}) |
| 55 |
> continue |
| 56 |
> # For each short lingua that isn't in LANGS, |
| 57 |
> # We used to add *all* long LANGS to the linguas list, |
| 58 |
> # but we stopped doing that due to bug 325195. |
| 59 |
> fi |
| 60 |
|
| 61 |
indentation on these comments seem to be off |
| 62 |
|
| 63 |
> # FIXME: Add support for unpacking xpis to portage |
| 64 |
> xpi_unpack "${MOZ_P}-${x}.xpi" |
| 65 |
|
| 66 |
or, add it to the new unpacker.eclass ;) |
| 67 |
|
| 68 |
also, seems to be missing `use linguas_${x} && xpi_unpack ...` ? otherwise, |
| 69 |
you just unpack all linguas and not just the ones the user has requested ... |
| 70 |
same goes for the install ... |
| 71 |
|
| 72 |
> if [[ "${linguas[*]}" != "" && "${linguas[*]}" != "en" ]]; then |
| 73 |
> einfo "Selected language packs (first will be default): ${linguas[*]}" |
| 74 |
|
| 75 |
since linguas[*] will be big by default, i'd put the variable expansion into |
| 76 |
its own einfo |
| 77 |
-mike |
Archives