-
Notifications
You must be signed in to change notification settings - Fork 29
refactor: webhook notifier re-design #314
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
Conversation
…ling in WebhookNotifier
…date Webhook tests to use NotificationService for formatting
…k payload builders
…ype logic and payload building
…oadBuilder for payload creation
… methods to accept templates and variables
…streamline webhook component creation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #314 +/- ##
=======================================
- Coverage 96.3% 96.2% -0.1%
=======================================
Files 78 76 -2
Lines 25229 24771 -458
=======================================
- Hits 24308 23847 -461
- Misses 921 924 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 @isSerge 🥇
The code LGTM, I just need to test it, I'll get it done by EOW.
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.
LGTM!
I have tested all channels, working well.
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.
LGTM, thanks for testing Nico
Summary
Closes #283
Mostly refactoring here:
discord,slackandtelegramnotifiersNotifiertraitPayloadBuildertrait that encapsulates logic to format message and build JSON for a specific channelAsWebhookComponentstrait forTriggerTypeConfigto consolidate all logic to create webhook config, choose payload builder, etc.Testing Process
cargo runChecklist