THREESCALE-9183: Removing leftovers of source2swagger #3327
Conversation
mayorova
changed the title
There was a problem hiding this comment.
I'm all for this cleanup. If the tests pass, approved. Just one thing: the comments removed here contain a list of accepted parameters for each endpoint which I find useful, shouldn't we keep the list?, no matter the format.
The idea is to replace source2swagger by rswag in the future. If I'm not wrong, I'd say in rswag you need to write specs where you define the interface for the endpoint, so we'll probably need the list of parameters in order to write those specs.
| @@ -250,8 +250,6 @@ group :development, :test do | |||
| gem 'pry-shell' | |||
| gem 'pry-stack_explorer' | |||
|
|
|||
| # for `rake doc:liquid:generate` and similar | |||
There was a problem hiding this comment.
Is this relevant? Should we remove/edit this task?
There was a problem hiding this comment.
I think this was a "typo" and it referred to doc:swagger:generate which I already removed.
There was a problem hiding this comment.
Do we still need swagger-ui_rails?
There was a problem hiding this comment.
Do we still need
swagger-ui_rails?
I'm afraid we do. I believe this is for Swagger v1, and we still support rendering Swagger UI for v1 specs for our customers.
mayorova
force-pushed
the
cleanup-old-swagger-annotations
branch
from
99d0d41
to
7e6e7a1
Compare
mayorova
force-pushed
the
cleanup-old-swagger-annotations
branch
from
31c95c3
to
24d3dd9
Compare
| def missing_config | ||
| false | ||
| end | ||
| end |
There was a problem hiding this comment.
Apparently this leftover was not used anymore. But what if old customers have this rolling update in their custom configuration? Would then their upgrade fail? Should we keep this forever or document that it needs to be removed before upgrade?
There was a problem hiding this comment.
You mean if they have new_provider_documentation: true in their rolling_updates.yml.
Well, they are not supposed to touch this configuration in the first place :)
Secondly, why would they do that? Just to load a broken Swagger v2 Active Docs for 3scale APIs? :)
And most importantly, the rolling_updates.yml can contain extra features, and if there is no code to process them, then they would be just ignored.
There was a problem hiding this comment.
hmm, I think it failed for me like that in the past, maybe don't remember. But it makes sense, nobody would have used v2.
| @@ -299,7 +299,6 @@ def call(env) | |||
|
|
|||
| namespace :api_docs do | |||
| resource :account_data, :only => [:show] | |||
| resources :specs, only: :show | |||
There was a problem hiding this comment.
This was a controller to load Swagger 2.0 specs:
/p/admin/api_docs/specs/finance.json
/p/admin/api_docs/specs/analytics.json
/p/admin/api_docs/specs/accounts.json
The controller itself app/controllers/provider/admin/api_docs/specs_controller.rb was also deleted.
* Removing leftovers of source2swagger * Remove circleci job for swagger generation * More source2swagger removals * More cleanups * Fix CircleCI config
What this PR does / why we need it:
Cleaning up old source2swagger annotations and related code.
Which issue(s) this PR fixes
https://issues.redhat.com/browse/THREESCALE-9183
Verification steps
-none-
Special notes for your reviewer:
Based on #3103