-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2SummitScheduleSettingsApiController
#402
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
Feature | Extend Swagger Coverage for controller OAuth2SummitScheduleSettingsApiController
#402
Conversation
18c20da to
c53b530
Compare
836dcb0 to
dbbdd75
Compare
caseylocker
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.
@matiasperrone-exo only one issue with the namespace.
Wrong Namespace in Security Schema
File: app/Swagger/Security/SummitScheduleSettingsAuthSchema.php
namespace App\Swagger\Security; // WRONG
Should be:
namespace App\Swagger\schemas; // CORRECT
While you're in there is there any reason that you're using numerical responses missed with constants in responses? E.g.
responses: [
new OA\Response(response: 204, description: 'No Content'), // <- Why?
new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
We should stick with one or the other.
|
Thanks @caseylocker for the comments, is now ready for review. |
caseylocker
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.
Looks good. @smarcet if you agree please merge.
0bd9d71 to
ab40dc1
Compare
|
@matiasperrone please re review the openapi doc generation is failing and there are some confiicts |
ab40dc1 to
21a2997
Compare
|
@matiasperrone-exo the issue looks like it's in SummitSchemas.php. The schema parameter in OA\Property expects a string (schema name reference), not an OA\Schema object. |
…tingsApiController`
21a2997 to
a88814a
Compare
smarcet
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.
LGTM
Task:
Ref: https://app.clickup.com/t/86b6wkh7d