Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use $Logger in cronjobs #494

Merged
merged 7 commits into from
May 5, 2024
Merged

use $Logger in cronjobs #494

merged 7 commits into from
May 5, 2024

Conversation

rjbs
Copy link
Collaborator

@rjbs rjbs commented May 4, 2024

Now that the cron jobs are all(?) successfully loading PAUSE::Logger, we should use it. This replaces "warn" calls with use of $Logger.

@rjbs rjbs requested a review from rspier May 4, 2024 15:07
@rjbs rjbs added the logging PAUSE::Logger, syslog, journald… label May 4, 2024
@rjbs rjbs marked this pull request as ready for review May 4, 2024 15:30
Copy link
Collaborator

@rspier rspier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

github's diff is not doing a great line displaying this change.

@@ -80,7 +80,7 @@ sub read_errorlog {
close LOG;
report "\nErrorlog contains $errorlines lines today\n";
} else {
warn "error opening $PAUSE::Config->{HTTP_ERRORLOG}: $!";
$Logger->log("error opening $PAUSE::Config->{HTTP_ERRORLOG}: $!");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is an error, should it be fatal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the whole premise of this code is kinda wonky, so I'm gonna not touch it beyond changing its logging. "Send me the day's web error logs" is odd.

@@ -241,7 +241,8 @@ sub watch_files {
quoteHighBit => 1,
);
my $v = $d->stringify($File::Find::name);
warn sprintf qq{Found a bad directory v[%s], rmtree-ing}, $v;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary blank line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most blanks are, right? :)

Anyway, so much of the PAUSE code is without any vertical whitespace. Sometimes I add some to help break things up. This is maybe not the most obvious place to put it, but: I will continue to add vertical whitespace to create paragraphs of code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like vertical whitespace, this just felt like a weird place to put it.

@rjbs rjbs merged commit 735c8f3 into andk:master May 5, 2024
1 check passed
@rjbs rjbs deleted the cron-logger branch May 12, 2024 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logging PAUSE::Logger, syslog, journald…
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants