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

logging: don't log if opt_in_channels is empty or channel not in it #7

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

RhinosF1
Copy link

@RhinosF1 RhinosF1 commented Dec 20, 2020

Add an opt_in_channels config which blocks logging if the array is not empty and the channel is not in it. Assumes if array is empty that logging should happen everywhere.

resolves #6

Add an opt_in_channels config which blocks logging if the array is not empty and the channel is not in it. Assumes if array is empty that logging should happen everywhere.
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Instead of repeating message.sender.is_channel() and bot.config.chanlogs.opt_in_channels in not [] and trigger not in bot.config.chanlogs.opt_in_channels everywhere, make a helper function like _channel_opted_in() and use that.

I'm also fairly sure that that set of conditions won't actually do what you want. Logically, it's gibberish. Probably won't even parse; doesn't look like valid Python.

sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
@dgw dgw added this to the 0.3.0 milestone Dec 20, 2020
@RhinosF1 RhinosF1 requested a review from dgw December 21, 2020 10:19
@RhinosF1
Copy link
Author

Update?

@dgw dgw changed the title logging: don't log is opt_in_channels is empty and channel not in array logging: don't log if opt_in_channels is empty or channel not in it Jan 28, 2021
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

There are so many syntax errors in this, I'm positive it hasn't been tested. 🙁

@@ -111,6 +112,17 @@ def _format_template(tpl, bot, trigger, **kwargs):
return formatted


def _channel_is_opted_in(is_channel, channel, opted_in_channels):
Copy link
Member

Choose a reason for hiding this comment

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

Why does this take is_channel as an argument? Callers derive it from Trigger.sender.is_nick() anyway, so… that can be part of the function.

Also: sopel.tools.Identifier, of which Trigger.sender is an instance, doesn't have an is_channel() method. Did you…test this patch?

@@ -111,6 +112,17 @@ def _format_template(tpl, bot, trigger, **kwargs):
return formatted


def _channel_is_opted_in(is_channel, channel, opted_in_channels):
if is_channel and not opted_in_channels or str(channel) in opted_in_channels: # cast channel to string because I don't trust python
Copy link
Member

Choose a reason for hiding this comment

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

channel should be cast to sopel.tools.Identifier, not str.

Also PEP8 demands two spaces before # to start a line-end comment.

Copy link
Author

Choose a reason for hiding this comment

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

Won't everything in the array be a string though?

sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
sopel_modules/chanlogs/__init__.py Outdated Show resolved Hide resolved
@dgw dgw removed this from the 0.3.0 milestone Jun 21, 2024
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.

Allow plugin to be only enabled on a per channel namespace
2 participants