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

add message reactions #459

Merged
merged 5 commits into from
Feb 21, 2018
Merged

add message reactions #459

merged 5 commits into from
Feb 21, 2018

Conversation

anxodio
Copy link
Contributor

@anxodio anxodio commented Feb 18, 2018

Description

This PR add reaction capabilities to opsdroid, adding a new react method in Message and Connector.

By default it just log a debug message and return False, but can be implemented by connectors that have react capabilities (like Slack or Github).

Fixes #442

Status

READY

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

(By now it will be documented in the Slack connector, because it will be the only one that implements it)

How Has This Been Tested?

I tested and checked that the new react methods returns False as expected.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@codecov
Copy link

codecov bot commented Feb 18, 2018

Codecov Report

Merging #459 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #459   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          18     18           
  Lines        1095   1104    +9     
=====================================
+ Hits         1095   1104    +9
Impacted Files Coverage Δ
opsdroid/connector.py 100% <100%> (ø) ⬆️
opsdroid/message.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b45cb64...140b38a. Read the comment docs.

@jacobtomlinson
Copy link
Member

jacobtomlinson commented Feb 18, 2018

This looks great!

You're missing a test for the case where thinking-delay is set in the config. As there is no else statement you could always just add it to the config on the connector in test_react().

Copy link
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

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

The code looks good and the tests are passing now. So it just needs adding to the docs and it's good to go!

@anxodio
Copy link
Contributor Author

anxodio commented Feb 21, 2018

Sorry, but i'm not sure of where to put this in the documentation. As I writed:

(By now it will be documented in the Slack connector, because it will be the only one that implements it)

And the documentation is in the other PR: opsdroid/connector-slack#10

So, if we have to add it here, where you would add it?

@FabioRosado
Copy link
Member

Since this is a new method of the connector class I think the right place for it would be in the connector documentation

Hope this helps 👍

@jacobtomlinson
Copy link
Member

Awesome many thanks!

@jacobtomlinson jacobtomlinson merged commit c7f2577 into opsdroid:master Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants