Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

@matiasperrone-exo matiasperrone-exo self-assigned this Oct 14, 2025
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Oct 14, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitschedulesettingsapicontroller branch from 18c20da to c53b530 Compare October 14, 2025 17:57
@matiasperrone-exo matiasperrone-exo added the review Need reviewing from the developer label Nov 10, 2025
@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 18, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitschedulesettingsapicontroller branch from 836dcb0 to dbbdd75 Compare November 26, 2025 15:14
Copy link

@caseylocker caseylocker left a 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.

@matiasperrone-exo
Copy link
Contributor Author

Thanks @caseylocker for the comments, is now ready for review.

Copy link

@caseylocker caseylocker left a 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.

@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitschedulesettingsapicontroller branch from 0bd9d71 to ab40dc1 Compare December 5, 2025 14:58
@smarcet
Copy link
Collaborator

smarcet commented Dec 8, 2025

@matiasperrone please re review the openapi doc generation is failing and there are some confiicts

@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitschedulesettingsapicontroller branch from ab40dc1 to 21a2997 Compare December 8, 2025 21:31
@caseylocker
Copy link

@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.
Remove the unnecessary wrapper OA\Property with the nested schema: parameter and define the properties directly on the SummitScheduleConfig schema

@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitschedulesettingsapicontroller branch from 21a2997 to a88814a Compare December 10, 2025 19:41
Copy link
Collaborator

@smarcet smarcet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smarcet smarcet merged commit ba85a22 into main Dec 16, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants