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

Use swagger-ui-dist instead of swagger-ui #115

Merged
merged 2 commits into from Jun 5, 2019
Merged

Use swagger-ui-dist instead of swagger-ui #115

merged 2 commits into from Jun 5, 2019

Conversation

thesocialdev
Copy link
Contributor

Based on hyperswitch implementation.

Bug: T218217

Based on hyperswitch implementation. See
wikimedia/hyperswitch@a040a59

Bug: T218217
@Pchelolo
Copy link
Contributor

  1. Let's bump minor version
  2. For this PR, let's also remove node from .travis.yaml as a version, node 12 just released and nothing works with it, fixing node 12 support will be a separate project.

@d00rman
Copy link
Contributor

d00rman commented Apr 29, 2019

  1. Let's bump minor version

Actually, let's not do that just yet, let's get rid of the optional params first too.

  1. For this PR, let's also remove node from .travis.yaml as a version, node 12 just released and nothing works with it, fixing node 12 support will be a separate project.

@Pchelolo
Copy link
Contributor

Actually, let's not do that just yet, let's get rid of the optional params first too.

We should do it as a part of the same PR actually..

@d00rman
Copy link
Contributor

d00rman commented Apr 29, 2019

We should do it as a part of the same PR actually..

Or that, yes, even better. @mateusbs17 could you adjust the routes in spec.template.yaml and split it in two (one with and the other without the extra param), please? Also, please do it in the same commit that bumps the version.

@thesocialdev
Copy link
Contributor Author

We should do it as a part of the same PR actually..

Or that, yes, even better. @mateusbs17 could you adjust the routes in spec.template.yaml and split it in two (one with and the other without the extra param), please? Also, please do it in the same commit that bumps the version.

Ok.

One question, should we remove specific code that checks for swagger 2.0 spec versions and keep only openapi 3?

@Pchelolo
Copy link
Contributor

Hm I see in MCS patch you are switching to openApi 3 right away. I think given that this is a template, let's be bold and completely switch to openapi 3 here too? What do you think @d00rman ?

@d00rman
Copy link
Contributor

d00rman commented Apr 29, 2019

let's be bold and completely switch to openapi 3 here too?

+1, let's do it. Apart from the optional path param, that we need to get rid of anyway, there's nothing that ties us to v2 in the spec template.

.travis.yml Show resolved Hide resolved
app.js Outdated Show resolved Hide resolved
1) Removing 'node' from travis.yml, support node 10 instead of node 8
2) Support only openapi 3 spec
3) Update spec.example.yaml to be openapi 3 compliant
4) Bump minor version
@@ -256,7 +256,7 @@ describe('Swagger spec', function() {
defParams = spec['x-default-params'];
}
// check the high-level attributes
['info', 'swagger', 'paths'].forEach((prop) => {
['info', 'openapi', 'paths'].forEach((prop) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use openapi-schema-validator in this test just like RESTBase does.

@Pchelolo Pchelolo dismissed d00rman’s stale review June 5, 2019 07:51

The comments have been addressed.

@Pchelolo
Copy link
Contributor

Pchelolo commented Jun 5, 2019

I'm gonna merge this and do a couple of followups

@Pchelolo Pchelolo merged commit 9c26670 into wikimedia:master Jun 5, 2019
wmfgerrit pushed a commit to wikimedia/mediawiki-services-mobileapps that referenced this pull request Jul 2, 2019
1) Change tests and app configuration for openapi 3.0 compliance
2) PR to upstream service-template-node, see
wikimedia/service-template-node#115
3) Use response and examples components to avoid code duplication
4) Change how spec.js tests x-amples on the new specification
5) Rebase + update specs for talk, media-list, and mobile-section-lead
6) Allow components to be added in the endpoint file and do a 2 level
depth merge only in the components key

Bug: T218220
Change-Id: Ibea93c3b790f2624f63380a8bcde95221f438ced
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants