Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

Adds ircbot command to ping staff in lab #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NotRyan
Copy link
Contributor

@NotRyan NotRyan commented Feb 2, 2019

Adds a command to ping all staff in the lab.

Currently bound to @labstaff, which will trigger Create to repeat the message, with your name at the front, and a list of labstaff's nicks where you wrote @labstaff. An example can be seen below.
screenshot_2019-02-01_17-39-43

Suggest any formatting changes here.

Note that this does not currently do a proper slack ping. Normally when a user in irc types out a xxx-slack username, its converted to a @xxx on Slack, but that doesn't seem to happen when this command is used to make the bot type it. I'm not entirely sure why that happens.

@dkess
Copy link
Member

dkess commented Feb 2, 2019

I am not sure this could work, for two reasons:

  • It's a bit invasive to ping everyone like this. However I block mentions from create anyways so if everyone else is fine with it then it's ok.
  • Slack/IRC names don't necessarily correspond to OCF usernames, so this will miss certain people.

@abizer
Copy link
Member

abizer commented Feb 3, 2019

we did turn this off for create: who is in the lab (bccb5ba) on purpose

@kpengboy
Copy link
Member

kpengboy commented Feb 3, 2019

It's a bit invasive to ping everyone like this

Eh...IMO sometimes it's useful, if an issue does require attention from someone physically present in the lab...

@abizer
Copy link
Member

abizer commented Feb 3, 2019

call it 'weewooweewoo' :P

@NotRyan
Copy link
Contributor Author

NotRyan commented Feb 3, 2019

Pinging with create: who is in the lab is spam because that command's purpose is to see who is in the lab. Pinging them would just be a side effect of that, serves no purpose, and ends up being spam because its an incidental ping that would happen needlessly. This, on the other hand, is a dedicated ping button. There is no reason that anyone would need to use this other than to ping people. It would only end up as spam if someone chooses to spam it.

As for the "invasiveness", I don't see how this can be considered invasive at all. It's pinging people by a public mention of their nick in a public irc channel. Any user can achieve the same exact thing by typing out a name. If being contacted through this is a serious concern for a user, there's always the option of blocking mentions from create as dkess brought up.

Copy link
Member

@cg505 cg505 left a comment

Choose a reason for hiding this comment

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

I think this would be useful, and I agree that it wouldn't really be spammy. The code is also pretty clean. lgtm

staff_nicks = staff_nicks | {nick for nick in channel_users if nick.startswith(staff_username)}
nicks_string = ', '.join(sorted(staff_nicks))

msg.respond(msg.text.replace("@labstaff", nicks_string))
Copy link
Member

Choose a reason for hiding this comment

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

why not just send the nicks_string instead of returning the replacement?

Copy link
Contributor Author

@NotRyan NotRyan Mar 19, 2019

Choose a reason for hiding this comment

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

My reasoning was that so people being pinged could see the actual message that the ping was for in the notification itself, eg. phone users could see it without having to actually open it up. However, its true that it does bloat things a bit.

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

Successfully merging this pull request may close these issues.

6 participants