-
Notifications
You must be signed in to change notification settings - Fork 4
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
Points can be added to emoticons #33
Comments
haha, I can see how surprising it may be :) thanks for bringing this up! it's actually a feature, more than a bug (yes, classic words) - we have emoticons for some of our team members, and then we just add +1 to those which is visually nice. the classical emoticons, like 😄 is probably just a side-effect of that in the current shape we allow adding +1 to any string. like for example we "+1" people who are not on our slack, but we still want to appreciate them how do you see it? is it something we should change? |
I can see value in rating emoticons :) So, since we can see it both as a bug and a feature, maybe adding option to allow/forbid is the best bet? |
it might be worth considering would you be for forbidding it for the DevsPL slack? I'm not against it, but I'm not sure it's worth the effort - it brings fun, it's quite useful. What would be the reason to forbid it for any organisation? meanwhile, I'll document on README, that this is a feature |
I would just disable it for normal/build-in emots (it's still looking like a bug for me) to avoid confusion. Rating custom emots can be quite helpful like you said. If it is worth effort? Depends - normally no one will use +1 for build-in emots, so there is no real need to disable it. From the other hand - if it's quick fix (maybe ignore list?) then making this a little bit more user friendly won't hurt. |
There's a related idea in here: #32 - aliasing the emoticons so that they can be counted as the person it represents. If we go with the aliases idea (I like it too), then some edge cases need to made sure - can we alias a built-in emot? what if it's forbidden? If anyone's up for contributing this option to forbid, I'd be happy to discuss/review the code changes. |
This class would be a good start to look at: https://github.com/arkency/plusone/blob/ad1d2e06626f4a6ae427912d60b49aeb11014bae/app/services/prepare_transaction_actors.rb |
I think this should not be possible ;)
The text was updated successfully, but these errors were encountered: