Re: Updated non-US.diff
On Thu, Jun 15, 2000 at 10:53:20AM +0100, Raphael Hertzog wrote:
>
>Le Wed, Jun 14, 2000 at 08:25:45AM +0100, Steve McIntyre écrivait:
>> OK, I believe I've covered all of Raphael's changes. This one now
>> works fine with !complete.
>
>Ok, you may commit it. I've seen that there's no problem with the
>generation of Debian_$CODENAME_nonUS since it's generated in $BDIR and
>not in the tasks dir.
>
>I just have 2 remarks :
>- I can't generate only one (US or non-US) first CD with
> FORCENONUSONCD1... but that's not a big deal since we don't gain much
> by beeing able to do this ...
Exactly...
>- this code may need a little tweak :
>
>> + foreach (@nonuslist) {
>> + if($complete or $includesrc{$_}) {
>> + msg(1, "Asked to add non-US source $_ to source CD#1\n");
>> + $src = $_;
>> + }
>> + if (not exists $included{$src}) {
>> + msg(0, "ERROR: Non-US source `$src' does not exist ...
>(ignored)\n");
>> + next;
>> + }
>> + if($excluded{$src}) {
>> + msg(1, "...but not doing so - $excluded{$src}\n");
>> + next;
>> + }
>> + next if $included{$src};
>> + add_src ($src);
>> + }
>
>I'd prefer that you use "foreach $src (@nonuslist)" directly instead of
>doing a conditionnal assignement, and you'd better do a next directly if
>the package don't have to be included. Otherwise if the assignment is not
>done, $src has still the old value and some code maybe executed twice (of
>course the included check will fail but that's not always self-evident for
>someone reviewing the code).
Hmmm, yes.
>Something like that should do it (I hope, i'm a bit tired right now ...) :
>foreach $src (@nonuslist)
>{
> if (! ($complete or $includesrc{$src}))
> {
> msg(1, "Non-US source package $src doesn't need to be included
>!\n");
> next;
> }
> msg(1, "Asked to add non-US source $src to source CD#1\n");
> if (not exists $included{$src}) {
> msg(0, "ERROR: Non-US source `$src' does not exist ...
>(ignored)\n");
> next;
> }
> if ($excluded{$src}) {
> msg(1, "...but not doing so - $excluded{$src}\n");
> next;
> }
> next if $included{$src};
> add_src ($src);
>}
OK, seems fine to me. I'll play with it tonight.
>Apart that everything looks ok, feel free to commit it with this litlle
>change. And just add a line about it in debian/changelog (and also update
>debian/CONF.sh).
Will do.
--
Steve McIntyre, Allstor Software smcintyr@allstor-sw.co.uk
Also available from steve_mcintyre@geocities.com
"Can't keep my eyes from the circling sky,
"Tongue-tied & twisted, Just an earth-bound misfit, I..."
Reply to: