-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Restructure webhook
module
#22256
Restructure webhook
module
#22256
Conversation
Previously, there was an "import services/webhooks" inside "modules/notification/webhook". This import was removed (after fighting against many import cycles). Additionally, a few constants were extracted from "models/webhooks" to "modules/notification/webhook".
I think |
What problem does this PR solve? The notification system depends heavily on other parts of our codebase and should therefore be moved into the services packages. It's not a "module" in our definition. |
Yes. That's also what I did with the former As to what problem this PR solves
So, overall, I simply untangled our way too complicated import structure slightly. By the way, I can spoiler you that I opened this PR due to a comment I made on my unreleased PR review for #21937. |
I think this is a right direction. |
Hmm, why does the CI keep failing consistently at the same location? |
As an aside I think notification belongs in services not modules. It clearly depends on models and therefore should be in services. |
import services
from webhook
modulewebhook
module
Codecov Report
@@ Coverage Diff @@
## main #22256 +/- ##
==========================================
- Coverage 48.15% 47.58% -0.57%
==========================================
Files 1043 1044 +1
Lines 142324 142432 +108
==========================================
- Hits 68536 67777 -759
- Misses 65608 66551 +943
+ Partials 8180 8104 -76
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
make L-G-T-M work |
* upstream/main: Add deprecated warning for DISABLE_GRAVATAR and ENABLE_FEDERATED_AVATAR (go-gitea#22318) Unify hashing for avatar (go-gitea#22289) fix: code search title translation (go-gitea#22285) Update Gmail mailer configuration (go-gitea#22291) Fix due date rendering the wrong date in issue (go-gitea#22302) Fix get system setting bug when enabled redis cache (go-gitea#22295) Restructure `webhook` module (go-gitea#22256) Reminder for no more logs to console (go-gitea#22282) Fix bug of DisableGravatar default value (go-gitea#22296) Upgrade go-chi to v5.0.8 (go-gitea#22304) [skip ci] Updated licenses and gitignores Use ErrInvalidArgument in packages (go-gitea#22268) Changelog v1.18.0 (go-gitea#22215) (go-gitea#22269) Support estimated count with multiple schemas (go-gitea#22276) Add Gentoo to the from package providers (go-gitea#22284) Fix sitemap (go-gitea#22272) Add `sync_on_commit` option for push mirrors api (go-gitea#22271) Fix key signature error page (go-gitea#22229)
Previously, there was an
import services/webhooks
insidemodules/notification/webhook
.This import was removed (after fighting against many import cycles).
Additionally,
modules/notification/webhook
was moved tomodules/webhook
,and a few structs/constants were extracted from
models/webhooks
tomodules/webhook
.