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

cleanup: Drop support for OpenAPI v2 #574

Merged
merged 1 commit into from Nov 10, 2023

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Nov 8, 2022

Problem

As mentioned in #503, maintaining OpenAPI v2 specification is becoming too big overhead to support.

Fixes #609

Solution

This PR is intended to drop all code related to OpenAPI v2 and end its support.

Also, because the Swagger has been renamed to OpenAPI, the codebase and documentation is also being updated in order to avoid confusion in terminology.

This concerns this parts of the OpenAPI Specification:

The changes I made are compatible with:

  • OAS2
  • OAS3
  • OAS3.1

Related Issues

Checklist

  • Added tests
  • Changelog updated
  • Added documentation to README.md

Steps to Test or Reproduce

@romanblanco romanblanco force-pushed the drop-openapi-v2 branch 3 times, most recently from 6d7b3f0 to 61ecf12 Compare November 9, 2022 10:09
@romanblanco romanblanco marked this pull request as ready for review November 10, 2022 20:32
@romanblanco romanblanco marked this pull request as draft November 10, 2022 20:32
@romanblanco romanblanco force-pushed the drop-openapi-v2 branch 12 times, most recently from e0a416c to 54ca72c Compare January 18, 2023 20:19
@romanblanco romanblanco force-pushed the drop-openapi-v2 branch 2 times, most recently from f4cdcba to ba1f6c6 Compare January 20, 2023 23:42
@romanblanco romanblanco added this to the Gem 3.0.0 milestone Mar 18, 2023
@romanblanco romanblanco force-pushed the drop-openapi-v2 branch 7 times, most recently from 8dc7528 to 389382c Compare April 1, 2023 21:48
@mattpolito
Copy link
Collaborator

This is a long running branch with a lot of changes... where are we at on this one? I'm guessing we'll have to do another pass since there are changes in master that appear to be removed here.

@romanblanco
Copy link
Member Author

romanblanco commented May 3, 2023

This is a long running branch with a lot of changes... where are we at on this one?

@mattpolito, besides responding to the review suggestions (renaming swagger_dry_run, starting MIGRATION_GUIDE.md,...), I think the changes here are not far from being done.

However, together with this, in 3.0.0 we also need to take care of updating other deprecations (Ruby, Rspec). It would be better to have the other changes ready as well (see #625, #636), when we decide to merge this one.

I'm keeping the PR updated with latest merges in the meantime.

@romanblanco
Copy link
Member Author

romanblanco commented Jul 21, 2023

@mattpolito @jtannas, I consider this change ready.

I'm wondering about what should we put into the migration document.

The summary of this PR would probably be

- "Swagger" has been rebranded to "OpenAPI"
  - `SwaggerFormatter` is now `OpenapiFormatter`
  - `swagger_root` is now `openapi_root`
  - `swagger_strict_schema_validation` is now `openapi_strict_schema_validation`
  - `swagger_helper` is now `openapi_helper`
  - `swagger_endpoint` is now `openapi_endpoint`
  - `swagger_filter` is now `openapi_filter`

  - `swagger_dry_run` is now `rswag_dry_run`  # !!!
  - `swagger_doc` is now `openapi_spec`  # !!!
  - `swagger_docs` is now `openapi_specs`  # !!!

  - The default folder for generating OpenAPI spec has moved from `swagger` to `openapi`.
  - `swaggerize` task has kept it's name

Other changes (e.g. moving #/definitions/ under #/components/schemas/) would be the migration of Swagger 2.0 to OpenAPI 3.0 itself.

Not sure we need to document that part here too 🤔.

@mattpolito
Copy link
Collaborator

Awesome I'll take a look this weekend to get a another comb through of it. If we're changing everything from swagger to openapi, I'd recommend us changing the task to something different as well and aliasing swaggerize to it.

@romanblanco romanblanco requested review from mattpolito and removed request for BookOfGreg July 31, 2023 06:16
Copy link
Collaborator

@mattpolito mattpolito left a comment

Choose a reason for hiding this comment

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

Have we considered the upgrade path for these changes? Dropping support for OpenAPI v2 is completely different than renaming so many things. For example has there been deprecation warnings put in place to say that swagger_root & swagger_endpoint are going away? That's going to break every install.

I know a lot of work has been put into this but the support for v2 should be dropped without renaming public api methods.

Then deprecation warnings put in saying that those methods will be changed in the next release.

If it comes along with a major version bump we could maybe throw caution to the wind but might be a bit bad form.

@romanblanco
Copy link
Member Author

romanblanco commented Oct 28, 2023

Have we considered the upgrade path for these changes? Dropping support for OpenAPI v2 is completely different than renaming so many things. For example has there been deprecation warnings put in place to say that swagger_root & swagger_endpoint are going away? That's going to break every install.

I know a lot of work has been put into this but the support for v2 should be dropped without renaming public api methods.

Then deprecation warnings put in saying that those methods will be changed in the next release.

If it comes along with a major version bump we could maybe throw caution to the wind but might be a bit bad form.

@mattpolito These changes would come along with a major version, but I agree. We can do better with letting folks know that we're renaming the methods in v3.

The PR is now aiming to be merged into v3 branch and I'll create a PR into current master, to warn about the change.

@romanblanco
Copy link
Member Author

@mattpolito, since these changes are going into v3 branch, I'll go ahead and merge to unblock other PRs.

The PR adding warning about upcoming changes in v3 is out: #688

@romanblanco romanblanco merged commit 5ac08b7 into rswag:v3 Nov 10, 2023
1 check passed
@romanblanco romanblanco mentioned this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropping of OAS2
3 participants