-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
Add Telegram Bot message reactions #146354
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 Telegram Bot message reactions #146354
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.
Hi @aviadlevy
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
b299506
to
faae606
Compare
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 tested this and it works fine.
38d9dca
to
415aa6f
Compare
@aviadlevy for the merge conflict, I think you only need to move your test to the end of the |
415aa6f
to
8f9cee1
Compare
I just rebased a few hours ago due to conflicts. |
8f9cee1
to
ec2ef5e
Compare
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.
Just 2 minor changes, but otherwise LGTM
Oh by the way, please avoid rebasing after getting a review and use merge instead. It makes it easier to review changes since Github does not deal with git history changes well for partial reviews. |
Thanks for the review! |
Yes. All commits will be squashed when merging to |
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.
Thanks!
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.
Thanks @aviadlevy !
By the way, for future PRs I would suggest avoiding adding unrelated changes (like updating other tests) in the same PR, as noted in https://developers.home-assistant.io/docs/review-process#creating-the-perfect-pr
Breaking change
Proposed change
Add message reaction to Telegram bot
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: