Mattermost integration update#30
Conversation
VladimirShitov
left a comment
VladimirShitov
left a comment
There was a problem hiding this comment.
This looks much better! Please address the comments
docs/index.md
Outdated
| @@ -1,3 +1,40 @@ | |||
| # paperbee | |||
There was a problem hiding this comment.
This file is not needed, please delete it. README.md in the root should be updated according to the other messengers' description
src/demo_mattermost.py
Outdated
| @@ -0,0 +1,57 @@ | |||
| import os | |||
There was a problem hiding this comment.
This file is not needed in the main repo, please delete it
tests/test_mattermost.py
Outdated
| def test_build_message(papers): | ||
| publisher = MattermostPaperPublisher( | ||
| logger=logging.getLogger(__name__), | ||
| url="dummy", token="dummy", team="dummy", channel="dummy", driver=MagicMock() |
There was a problem hiding this comment.
Don't use mocks and dummy IDs, we need a real test. If you don't have mattermost, read these variables from config, we will run it
CHANGELOG.md
Outdated
|
|
||
| ## 1.1.0 | ||
|
|
||
| - Added Mattermost integration: format and send daily paper digests to Mattermost channels. |
There was a problem hiding this comment.
Please make subtitles "Added", "Changed" and put bullet points in the corresponding lists
punithsaireddy
force-pushed
the
mattermost-integration-update
branch
from
e0da879 to
f3528cc
Compare
punithsaireddy
force-pushed
the
mattermost-integration-update
branch
from
f3528cc to
9bc405d
Compare
tests/test_slack.py
Outdated
| @pytest.mark.integration | ||
| def test_slack_integration(): | ||
| with open("files/config.yml") as f: | ||
| with open("files/config_template.yml") as f: |
There was a problem hiding this comment.
Please revert this back in all files. The idea is that the config template stays untouched (as a template), but a user or a tester has to create config.yml with the real variables
|
Thank you! This looks great to me. Please address the comment above on the |
VladimirShitov
left a comment
VladimirShitov
left a comment
There was a problem hiding this comment.
It is now working. Thanks, @punithsaireddy for your contribution!
2 participants
No description provided.