-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(mattermost): add support for icons #237
Conversation
Just add it anyway. The point is that you should be able to pass |
Co-authored-by: nils måsén <nils@piksel.se>
Thanks for such a quick response and even giving me the commit suggestion for the fix! 😄 |
Codecov Report
@@ Coverage Diff @@
## main #237 +/- ##
==========================================
+ Coverage 76.85% 77.07% +0.21%
==========================================
Files 95 95
Lines 2826 2844 +18
==========================================
+ Hits 2172 2192 +20
+ Misses 474 473 -1
+ Partials 180 179 -1
Continue to review full report at Codecov.
|
Any tips to get the codecov higher? |
Could I get some assistance on increasing the CodeCov please? |
You can use the generic service compliance tests: shoutrrr/pkg/services/bark/bark_test.go Lines 135 to 160 in 5bca245
|
|
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.
Great work! I changed the scheme in the test URL, it doesn't matter for the test, but it's confusing to have the name of another service in the URL.
@all-contributors add @JosephKav for code |
I've put up a pull request to add @JosephKav! 🎉 |
Icon support on MatterMost:
Which issues it solves
Moving my Argus project to use Shoutrrr. Really want to keep icon support!
Tests that verify the code your contributing
I tried adding tests (and most of this code is copied from the Slack service), but couldn't get them to pass. Every other service that has query params seems to include the
title
param, but that's no supported on MatterMost, so I'm not sure what to do w.r.t this error. Help fixing this would be appreciated!Done? Let me know if you'd like any more, or changes to them. (Hoping the 'The services does not support any query/param props' would go away with this current commit?)