Skip to content
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

Backward compatibility to OpenAPI v3.0 #84

Closed

Conversation

RobinTail
Copy link
Contributor

@RobinTail RobinTail commented Aug 19, 2022

This PR might fix the issue #83 by making several types and the builder class generic.
The generic types depend on the new type OpenAPIVersion that differentiates 3.0.x and 3.1.x.

Some properties like exclusiveMinimum and exclusiveMaximum become either boolean (3.0.x) or number (3.1.x) depending on the OpenAPI version.

Since the default OpenAPI version is 3.0.0 (though it can be determined automatically from the openapi parameter), these changes might be considered as breaking to the current major version of the library.

I'm open to any suggestions and recommendations on how I could improve this PR to support both 3.0 and 3.1.

Closes #83

@RobinTail RobinTail marked this pull request as ready for review August 19, 2022 19:15
@RobinTail
Copy link
Contributor Author

@pjmolina && @jonluca , please review

@pjmolina
Copy link
Contributor

Thanks for the proposal @RobinTail. Formally, it seems to me pretty correct.

However, I have a concern about the maintainability problems it can introduce in the long term if we decide to follow this path:

  • The PR impacts in many small places spreading type versioning info in all the object model.
  • For future changes, it can only get worse and growing and we will be forced to follow this pattern.
  • Someone can forget how to apply this pattern ruining it all, making the lib more fragile.

In this particular case, the issue was to fix exclusiveMaximum & exclusiveMinimum typing.

The pragmatic alternative (lest cost in terms of maintenance) is: to type both exclusiveMaximum & exclusiveMinimum as number | boolean to support both 3.0.x and 3.1.0. This is what I feel we should do here, so far.

The rationale here is, an Openapi Validator will validate and reports full errors on the document.
We can always pass the validator this later, but at building time we are no constraining both approaches.
Our goal here is to be able to build the doc (not necessarily to validate). The validator will tell us if valid (needed in any case because it will check for more constraints that the ones enforced by the type-systems).

Thoughts?

@RobinTail
Copy link
Contributor Author

@pjmolina ,

yes. That make sense. Thank you for sharing.
I'll prepare another PR according to the concept you described.

@RobinTail
Copy link
Contributor Author

Instead of this — #85

@RobinTail RobinTail closed this Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v3.0: exclusiveMaximum must be a boolean
2 participants