-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/http api new approach #348
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
9a3a6aa to
aa2b625
Compare
nilmerg
left a comment
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.
This review covers only the early parts of routing and dispatching.
|
Oh, and I got the postgres tests working in my environment. Please get back to me for this. |
1705125 to
fe816f8
Compare
sukhwinder33445
left a comment
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.
I have looked at channels and contact tests so far, next I will look at contactgroup tests.
sukhwinder33445
left a comment
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.
Please add following changes as well:
- The PHPDoc for the method must contain a newline between different annotations, i.e.
@param,@return. - Update method PHPDoc if required.
- Make the
getPlural()method protected. - Remove the resolved todos.
30b0b1b to
da665ca
Compare
nilmerg
left a comment
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.
Let's discuss how we handle the openapi description and the usage of Swagger, as there are three possible solutions for me:
- Caching (properly, not the way it's right now ;) )
- CI (as your todo suggests)
- Static (it's a versioned api after all)
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.
Missing license header. It seems, not only this file is missing it, please make sure all PHP files have one.
This reverts commit 340d101.
3353654 to
9376563
Compare
nilmerg
left a comment
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.
🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉
Initial setup for the new approach of the http-api with openapi description.
Resolves:
require:
external_uuidtocontact/contactgroup/channeltable icinga-notifications#216