-
Notifications
You must be signed in to change notification settings - Fork 35
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
Introduce reactions #428
Introduce reactions #428
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀👍
sendReaction(roomId, eventId, "👀"); | ||
if (!arg0) { | ||
logger.warn("Address not provided, skipping"); | ||
sendReaction(roomId, eventId, "😕"); | ||
sendMessage(roomId, `${sender} Please provide an address to be funded.`); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like there's no need to send 👀 reaction, if we're sending a 😕 right after.
Why not move sendReaction(roomId, eventId, "👀");
after synchronous checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, also thought about it.
In my pedantic mind it's more consistent to always have the same pattern:
seen 👀
->👍 OK
seen 👀
->😕 FAIL
Changing this would introduce a 😕 FAIL
for a message that "has not been seen" in contrast with others.
Those were just my thoughts, don't know if it makes sense :D
Note: investigating the failing E2E tests. Not on issue with the PR, they fail on clean master as well. Edit: Master fixed, now we have some actual failure on the PR. |
Analogous to how the tip bot works, introduced reactions.
The user will get immediate feedback that the command has been received.