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

Support custom webhook response #793

Closed
wants to merge 3 commits into from
Closed

Support custom webhook response #793

wants to merge 3 commits into from

Conversation

cupen
Copy link

@cupen cupen commented Dec 24, 2018

Description

sometimes, we need to respond a custom json or text to the webhook caller.
let me know what do you think about this. I'll add or run testcase later.

Status

UNDER DEVELOPMENT

Type of change

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

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 Dec 24, 2018

Codecov Report

Merging #793 into master will decrease coverage by 0.05%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #793      +/-   ##
==========================================
- Coverage     100%   99.94%   -0.06%     
==========================================
  Files          29       29              
  Lines        1901     1903       +2     
==========================================
+ Hits         1901     1902       +1     
- Misses          0        1       +1
Impacted Files Coverage Δ
opsdroid/web.py 98.71% <66.66%> (-1.29%) ⬇️

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 14e1530...8800c20. Read the comment docs.

@jacobtomlinson
Copy link
Member

Awesome! This is really useful, thanks for raising this. Please go ahead with tests and documentation.

@cupen
Copy link
Author

cupen commented Jan 6, 2019

OK. I'll do it.

jacobtomlinson added a commit that referenced this pull request Jan 12, 2019
* Support custom webhook response

* Add tests for custom webhook response

* Add docs for custom webhook response
@jacobtomlinson
Copy link
Member

I've gone ahead and added the tests and docs, I totally understand that things sometimes get in the way of updating a PR you've raised. It has now been merged in #807 so I'm going to close this PR.

Thanks for your help! Ping me your address via DM on Twitter or Gitter and I'll post you some opsdroid stickers!

jacobtomlinson pushed a commit that referenced this pull request Jan 12, 2019
* Make it possible to create class-based skills

* Rework based on requests in the PR

* Add deprecation warnings if a function based skill is loaded

* Update documentation with the details of the class based skills

* Add pylint exception and remark about a broad except in skill.py

* Add tests for the Skill base class

* wip: Add tests for the class-based skill loader

* WIP: Add tests for the class based skills

* Update pytest-cov from 2.6.0 to 2.6.1 (#799)

* Update pillow from 5.3.0 to 5.4.1 (#797)

* Update pytest from 4.0.2 to 4.1.0 (#798)

* Update arrow from 0.12.1 to 0.13.0 (#800)

* Update recommonmark from 0.4.0 to 0.5.0 (#802)

* Update aiohttp from 3.5.1 to 3.5.2 (#801)

* Update aiohttp from 3.5.2 to 3.5.3 (#804)

* Bump Pyyaml to avoid CVE-2017-18342 (#803)

* Bump Pyyaml to avoid CVE-2017-18342

* Add SafeLoader to yaml.load

* Add test for fix

* Update aiohttp from 3.5.3 to 3.5.4 (#805)

* Support custom webhook response (replacement for #793) (#807)

* Support custom webhook response

* Add tests for custom webhook response

* Add docs for custom webhook response

* Fix linting issues

* Fix broken test

* Add tests for class skills

* Update all documentation to use new class based skills

* Update class name
@cupen
Copy link
Author

cupen commented Jan 19, 2019

Nice work! sorry for my procratinating.

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