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

[irssi_logger.pl] added a blacklist option, added host and port as new options #853

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

terminaldweller
Copy link
Contributor

Like the title says, added a blacklist, host and port option.
tested with postgres 15.4 and 14.2 .

@ailin-nemui ailin-nemui changed the title added a blacklist option, added host and port as new options [irssi_logger.pl] added a blacklist option, added host and port as new options Aug 21, 2023
@ailin-nemui
Copy link
Contributor

question if the blacklist should be changeable without having to save and reload the script?

@ailin-nemui
Copy link
Contributor

general remark: version needs to be increased

@terminaldweller
Copy link
Contributor Author

i dunno how. lemme go read another script and fix that.
and yes. i should increase the version too.

@ailin-nemui
Copy link
Contributor

@qbit

}

sub write_db {
my ($nick, $message, $target) = @_;
my @vals;
my $date = strftime("%Y-%m-%d %H:%M:%S", localtime);
if (exists $blacklist{$target}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, i'd just do an if (!exists here and exclude the } else {.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor

@qbit qbit left a comment

Choose a reason for hiding this comment

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

LGTM

@ailin-nemui ailin-nemui merged commit 7354ec8 into irssi:master Aug 23, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants