-
Notifications
You must be signed in to change notification settings - Fork 88
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: add enforce
support for project hooks/webhooks configuration
#664
Conversation
If enforce==true in hooks config all non present hooks are deleted
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #664 +/- ##
==========================================
+ Coverage 84.27% 84.41% +0.14%
==========================================
Files 69 69
Lines 2741 2747 +6
==========================================
+ Hits 2310 2319 +9
+ Misses 431 428 -3
|
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.
Do we need an additional test case when hooks delete amd enforce is used together? I guess the existing test case named test_hooks_delete
kind of takes care of delete with enforce disabled. What happens delete for a hook is enabled and enforce is also enabled?
enforce
support for project hooks/webhooks configuration
Co-authored-by: amimas <aver.mimas@gmail.com>
Bug fix: iterator config_hooks to tuple (got consumed before 'enforce' section)
…orm into #662_enforce_hooks
Co-authored-by: amimas <aver.mimas@gmail.com>
Co-authored-by: amimas <aver.mimas@gmail.com>
In test_hooks, changed string split to group.path
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.
Looks good. Thanks again for this PR.
Added 'enforce' support for hooks processor. If set to true, all hooks not present in config yaml will be removed. Set to false is the same as omitting it - in this case 'delete: true' has to be set on any hook that should be deleted.
Added 2 tests, enforce to true/false.
Updated documentation section on webhooks.
Closes #662