Same as bug 28572 for $ENV{DEBUG} and $DEBUG.
Created attachment 122281 [details] [review] Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct
Created attachment 122321 [details] [review] Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Another nice bit of cleanup here. There are a few cases of "$debug and warn..." lines, but they appear to hard code setting `$debug = 0` internally and can be handled in another report I think. Signing off.
QA: Looking here now
Created attachment 122327 [details] [review] Bug 28620: Remove trailing space when logging with log4perl Trivial change. Do sed -i -r -e'/log4perl/ s/\s%n$/%n/' on the log4perl configs. Test plan: Update your own config. Trigger some logging and check that logfile. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Oops
Comment on attachment 122327 [details] [review] Bug 28620: Remove trailing space when logging with log4perl Not here
+#warn "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", join ' ', @mapkeys, "\n"; @mapkeys = grep {defined $mapping{$_}->{is}} @mapkeys; -$debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; +#warn "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; Any plans here ?
+ warn #"Finished \%memberhash has ", scalar(keys %memberhash), " keys\n", "Referencing \%mapping with ", scalar(keys %mapping), " keys\n"; Typo typo
Just a side note: - if ($pkt =~ /AZ(....)$/) { - $debug and warn "verify_cksum: sum ($1) detected"; - } else { + unless ($pkt =~ /AZ(....)$/) { I have something with unless. When the ifs go down and the unlesses go up, Koha gets more complicated ;)
-$ENV{DEBUG} = 1; -warning_like { in_iprange("192.168.1.1/36") } - qr/cidrlookup failed for/, - 'noisy simple invalid ip range/36 with remote ip in it'; Why removed?
Created attachment 122330 [details] [review] Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks good to me, noting a few minor points on BZ.
(In reply to Martin Renvoize from comment #3) > Another nice bit of cleanup here. > > There are a few cases of "$debug and warn..." lines, but they appear to hard > code setting `$debug = 0` internally and can be handled in another report I > think. Yes, it is expected. It's from scripts, not module. I thought it would be good to keep them. (In reply to Marcel de Rooy from comment #8) > +#warn "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", join ' ', > @mapkeys, "\n"; > @mapkeys = grep {defined $mapping{$_}->{is}} @mapkeys; > -$debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys > (populated): ", join ' ', @mapkeys, "\n"; > +#warn "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', > @mapkeys, "\n"; > > Any plans here ? I don't think they should be logged, but they sounded useful, in case someone need to debug LDAP. (In reply to Marcel de Rooy from comment #9) > + warn #"Finished \%memberhash has ", scalar(keys %memberhash), " keys\n", > "Referencing \%mapping with ", > scalar(keys %mapping), " keys\n"; > > Typo typo Oops, will fix. (In reply to Marcel de Rooy from comment #10) > Just a side note: > - if ($pkt =~ /AZ(....)$/) { > - $debug and warn "verify_cksum: sum ($1) detected"; > - } else { > + unless ($pkt =~ /AZ(....)$/) { > > I have something with unless. When the ifs go down and the unlesses go up, > Koha gets more complicated ;) I like unless :) (In reply to Marcel de Rooy from comment #11) > -$ENV{DEBUG} = 1; > -warning_like { in_iprange("192.168.1.1/36") } > - qr/cidrlookup failed for/, > - 'noisy simple invalid ip range/36 with remote ip in it'; > > Why removed? Heh, it was not trivial to keep as we are now using Koha::Logger, and didn't think very useful to keep anyway.
Created attachment 122331 [details] [review] Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks good to me, noting a few minor points on BZ. JD amended patch: replace "warn #Finished" with "#warn Finished", and put the statement on a single line
Pushed to master for 21.11, thanks to everybody involved!
t/Auth_with_shibboleth.t is failing 12:06:13 koha_1 | Log::Log4perl configuration looks suspicious: No loggers defined at /usr/share/perl5/Log/Log4perl/Config.pm line 325.
Enhancement not pushed to 21.05.x
Created attachment 122796 [details] [review] Bug 28606: Remove warning from C4::Auth 887 [WARN] Use of uninitialized value in sprintf at /kohadevbox/koha/C4/Auth.pm line 887.
Follow-up pushed to master.
*** Bug 28699 has been marked as a duplicate of this bug. ***