[Koha-bugs] [Bug 24153] Add a confirm flag to the cleanup_database.pl cronjob

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24153

--- Comment #13 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
(In reply to Jonathan Druart from comment #11)
> (In reply to Marcel de Rooy from comment #10)
> > +    $tokens->delete if $confirm;
> > +    say sprintf "Removed %s expired OAuth2 tokens", $count if $verbose;
> > This might be confusing. Among other examples.
> 
> What do you mean, can you detail? Do you mean the message could be improved
> (use of conditional) in dry-run mode?
Yeah, this holds in general. It might be confusing to say deleted when you are
in dry run mode.

> 
> > * perl misc/cronjobs/cleanup_database.pl -m 10 -v
> > Mail queue purge triggered for 10 days.
> > Use of uninitialized value $count in concatenation (.) or string at
> > misc/cronjobs/cleanup_database.pl line 293.
> >  messages were deleted from the mail queue.
> > 
> > So uninitialized warning that we do not want.
> > But this example could serve to illustrate that even the wrong count could
> > have been printed. Look at:
> >     if ( $confirm ) {
> >         $sth->execute($mail) or die $dbh->errstr;
> >         $count = $sth->rows;
> >     }
> >     print "$count messages were deleted from the mail queue.\nDone with
> > message_queue purge.\n" if $verbose;
> > => Since the count is not set, it could well be the count of a former
> > statement !
> 
> I fixed the warning.

Cool. But are you confident that it is the only location where it could happen?
Block X filled $count. And block Y presents it as a result while it actually
comes from X.

-- 
You are receiving this mail because:
You are watching all bug changes.