[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: localization-config v 0.113 into testing



Hi Konstantinos,

On Thu, Apr 28, 2005 at 12:03:48AM +0300, Konstantinos Margaritis wrote:
> Please consider letting the newer version of l-c into testing.
> It fixes a number of bugs (though nothing critical) but mainly it 
> introduces extensive logging in /var/log/localization-config.log. 
> This at least makes it worthwhile to have, IMHO.

A couple of the changes here look suspect to me:

--- /tmp/files0TieQ/localization-config-0.111/conffiles.d/common/editconfig.pl  Fri Feb  4 12:16:56 2005
+++ /tmp/fileonIWiw/localization-config-0.113/conffiles.d/common/editconfig.pl  Tue Apr 12 21:26:22 2005
@@ -24,8 +26,8 @@
     }
 
     if ($updated) {
-        print "Updating $filename\n" if $debug;
-        open(FILE, ">$filename.new") || die "Unable to write $filename.new";
+        log_msg("$0: Updating $filename");
+        open(FILE, ">$filename.new") || log_msg("$0: Unable to write $filename.new");
         print FILE @lines;
         close(FILE);
         rename "$filename.new", $filename;

Were the changes from fatal errors to log messages intentional?

--- /tmp/files0TieQ/localization-config-0.111/conffiles.d/gdm.postinst  Thu Feb  3 02:49:50 2005
+++ /tmp/fileonIWiw/localization-config-0.113/conffiles.d/gdm.postinst  Tue Apr 12 21:15:36 2005
@@ -9,16 +9,12 @@
 use warnings;
 use Getopt::Long;
 
-my $debug;
-
-# Get command line options
-GetOptions("debug" => \$debug);
-
 # Get lang entry
-my $lang = $ARGV[0] or die "No language given";
+my $lang = $ARGV[0] or log_msg("$0: No language given");
 
 # We use methods in common.pl
 require '/usr/lib/localization-config/common/common.pl';
+require '/usr/lib/localization-config/common/log.pl';
  
 # The path where the scripts are kept
 my $LIB = '/usr/lib/localization-config';

Has this been tested?  Perl doesn't let you call a function prior to the
'require' statement that defines it...  And again, the question of errors
vs. logs...

conffiles.d/woody/ispell also seems to have an encoding error in the
spelling of 'bokmål'.

I'm not willing to approve this package as it is right now; if you can
address the above issues (the die vs. log_msg issue is particularly
pervasive, and seems likely to have very bad unintended side effects), it
should be ok to let in.

Thanks,
-- 
Steve Langasek
postmodern programmer

Attachment: signature.asc
Description: Digital signature


Reply to: