https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24153 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #10)
+# FIXME The output for dry-run mode needs to be improved Yes. Agreed.
The following line is: # But non trivial changes to C4::Members need to be done before. So cannot be done now.
+ $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?
* 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.
I do not think that this is ready for production. Since we do not really support testing for many options, I do not see why this patch is useful.
I wrote it for the pseudonymized transactions. The idea was to rework a bit the whole script and introduce a common pattern. That's why I implemented filter_by_last_update and use it all over the script. We now have a base and a pattern to follow. It's not useful yet for all the options, but at least it is for few of them. -- You are receiving this mail because: You are watching all bug changes.