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

Add option to use PRIVMSG instead of NOTICE #1

Closed
wants to merge 2 commits into from
Closed

Add option to use PRIVMSG instead of NOTICE #1

wants to merge 2 commits into from

Conversation

ddevault
Copy link

@ddevault ddevault commented Jan 6, 2020

This is desirable in some situations.

I've also included a separate commit which updates this package to use Go modules.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@ddevault
Copy link
Author

ddevault commented Jan 6, 2020

Note: I'm not going to sign the CLA. My changes are available under the license in my fork, which is unchanged from your license. Take it or leave it.

@shammash
Copy link
Collaborator

Thank you Drew.

Leaving the licensing topic on the side for a second, I'd like to have a better understanding of the reason behind this request.

As far as I can see in the IRC RFC, NOTICE is typically used for automation because, unlike PRIVMSG, "automatic replies MUST NEVER be sent in response to a NOTICE message".

Could you provide a few examples of situations in which PRIVMSG should be used instead of NOTICE ?

Thank you,

Luca

@ddevault
Copy link
Author

Hey @shammash, on IRC the use of NOTICE or PRIVMSG is not quite as distinct as it once might have been. In practice they're not semantically different, but they do have practical differences which can make them better or worse. For example, for moderation - if you have a bot which use NOTICEs, you must enable NOTICEs for all users, including banned users. PRIVMSG can be moderated more effectively, however. Additionally, some users just prefer the UX of PRIVMSG over NOTICE in their client.

@ddevault
Copy link
Author

Oh, and if your channel has only one bot in it, or your bot's don't overlap, then it's not going to be a problem in practice, given the reasons you pointed out.

@shammash
Copy link
Collaborator

Thank you Drew for the additional details.

Now, given that signing the CLA is mandatory for contributing directly with a pull request, is it ok for you to consider this an issue / feature request?

I will reimplement the feature you are asking for.

Thank you,

Luca

@ddevault
Copy link
Author

Do as you like. Consider me a concientous objector to the CLA.

shammash added a commit that referenced this pull request Jan 25, 2020
Use a more generic name as there is soon going to be support for PRIVMSG
(see #1 for
background).

This introduces a backward-incompatible change in the config file for
these two parameters:
- notice_template -> msg_template
- notice_once_per_alert_group -> msg_once_per_alert_group

I am not introducing the new parameters with a deprecation plan since
both parameters are relatively secondary to the core functioning of the
bot (and this is a free time project after all).

Signed-off-by: Luca Bigliardi <shammash@google.com>
shammash added a commit that referenced this pull request Jan 25, 2020
Add `use_privmsg` config option to deliver alerts with PRIVMSG instead
of the default NOTICE.

This addresses a use case described in
#1 .

Signed-off-by: Luca Bigliardi <shammash@google.com>
@shammash
Copy link
Collaborator

Hi Drew,

ae6594c added use_privmsg .

I am going to close this PR. Please feel free to open an issue if you need other features.

Thank you,

Luca

@shammash shammash closed this Jan 25, 2020
@ddevault
Copy link
Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants