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

Privately notify a rated user if that user would not have seen it happen #64

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

Conversation

JustinTArthur
Copy link
Contributor

If you privately message your rating of a user to gribble, gribble will in turn privately message the user that was rated.

It still behaves as before if the rating happens in a channel that the rated user is in or if the rated user is not authed.

@teward
Copy link
Contributor

teward commented Jan 5, 2015

I also fail to see why we should have gribble PM a user every time they're rated. Also some questions:

  • Do you also verify whether the individual being messaged actually is online and authed?
  • Do you get the nickname of the active user on the specific auth/rating-target too?

And the big one: We occasionally have users who mass spam neg votes to people, usually targeting channel ops or people they don't like. In this system, do you take this into account and incorporate anti-spamflood protections to prevent the individual who's getting spam-neg-rated from getting inundated with PMs? (This is why I fail to see this should be a feature)

@JustinTArthur
Copy link
Contributor Author

Per a commit comment made by @teward, I've adjusted my pull request to use the same local variable nomenclature conventions employed throughout gribble and the supybot-bitcoin-marketmonitor plugins. My original intention had been to ensure compliance with the standard Python coding conventions. I am ok with sacrificing this in the name of consistency.

@teward
Copy link
Contributor

teward commented Jan 5, 2015

@JustinTArthur my other questions remain unanswered, and are extremely relevant. However, as any developer knows, while there may be standard $language_of_choice coding conventions, it is better to use consistency with what the application's code uses over using the nomenclature that is suggested by 'standard coding conventions'.

Still waiting to hear a response to my initial 3 questions made on the pull request here as well.

@JustinTArthur
Copy link
Contributor Author

No problem, @teward:

  • Do you also verify whether the individual being messaged actually is online and authed?
    • Yes. _checkGPGAuthByNick returns None when the user is not online or authed.
  • Do you get the nickname of the active user on the specific auth/rating-target too?
    • Yes, it's the only one we get the active user for in the code added by this PR. The nickname is derived from the hostmask in the GPG plugin instance's authed_users dict with ratedircnick = ircutils.nickFromHostmask(ratedhostmask)
  • …do you take this into account and incorporate anti-spamflood protections to prevent the individual who's getting spam-neg-rated from getting inundated with PMs?
    • Nope, and I'm open to ideas if this is a big concern. I will say, if a shill group is neg-rating me privately, I would definitely welcome some kind of notification of this activity. To me, this is a use case that further justifies this feature, aside from the pesky spam/flood problem.

@teward
Copy link
Contributor

teward commented Jan 5, 2015

I agree with your reasoning on the third part, but it gets very annoying if you get 20 PMs at once, and I'm not sure whether there's flood control in the bot or not - that being absolutely critical to verify in order to make sure that rating doesn't become an avenue for Denial of Service (by gribble sending many PMs at once and exploding). Pretty sure there's some level of flood control in gribs, but of course I want to make sure that's the case if we keep this feature.

@JustinTArthur
Copy link
Contributor Author

There is flood control in gribble, however it is just a queue throttling mechanism. All 20 of those messages would still get to the target user, they'd just get there slowly.

@JustinTArthur
Copy link
Contributor Author

Queried the channel for more feedback, got some mixed reviews, mostly negative:

st********: id say its a nice feature

gr*****: people can just join #bitcoin-otc-ratings
gr*****: and should be highlighted if rated

RB*****: annoying
RB*****: -otc-ratings
RB*****: and usually they're done in channel anyways
RB*****: plus, as teward mentioned, spambots

So I guess up to you, @nanotube. Feel free to close it out without merge, given current feedback.

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.

None yet

2 participants