-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add event in firewall / guard #251
base: v3.x
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## v3.x #251 +/- ##
============================================
- Coverage 92.50% 92.05% -0.45%
- Complexity 442 456 +14
============================================
Files 66 69 +3
Lines 1600 1648 +48
============================================
+ Hits 1480 1517 +37
- Misses 120 131 +11
Continue to review full report at Codecov.
|
This PR is ready to be merge, for the moment I've created a I've created this formatter to avoid break current user implementation on error handling (currently the error is return directly in the body with no formatting). |
@froozeify I don't have the time atm to fully review this PR. But just taking a quick look I can't help but notice the removal of the Also I'd prefer the |
@X-Coder264 Yes the removal of When I start developing the Event feature I was more thinking to add that in a major update but didn't find a v4.x (I should have asked...). And since it was my first PR on a public project forgot about it... (more used to work on company project with a continuously app evolution, so not respecting the Semantic Versioning don't have a major impact...) What I could propose, so the Event is in the v3.x code is:
tl;dr: I think the event system is awesome 😉 but it can only be added in the next major release and by removing at the same time the old For the For that part, I will remove it from this branch. If you still want it, I can move that code in a separate issue and PR so that custom formatting could be added in a minor v3.x release. (making some user like me happy, because I'd prefer having my frontend always getting JsonResponse, and not having to guess what the response format could be). More reasonably, I think we should not add it anymore. |
This reverts commit bb05bd6.
… in a major release)
… add-event-in-firewall-guard
…and wrong bearer)
…and wrong bearer)
…e wasn't catch) update: start updating Guard to new notification system
This PR aim to add support of Event inside the Firewall and Guard authenticator.
More details about the starting discussion here
What's this PR changes