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

Upgrade to swagger-ui v3 #76

Merged
merged 10 commits into from Sep 8, 2017
Merged

Upgrade to swagger-ui v3 #76

merged 10 commits into from Sep 8, 2017

Conversation

jplock
Copy link
Member

@jplock jplock commented Apr 17, 2017

The swagger v3 UI is very different than the v2 version. I'm not sure when it would be best to merge this in but my guess is v2 is no longer supported.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.967% when pulling 76e61c1 on jp-swagger-ui3 into 371211b on master.

Copy link

@jadams74 jadams74 left a comment

Choose a reason for hiding this comment

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

lgtm. html largely outside my wheelhouse though

@jplock
Copy link
Member Author

jplock commented May 9, 2017

Thanks for reviewing @jadams74. I need to update to the latest UI build.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2e0e77f on jp-swagger-ui3 into ** on master**.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.967% when pulling 2e0e77f on jp-swagger-ui3 into f80c7fe on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.967% when pulling 87dd7da on jp-swagger-ui3 into f80c7fe on master.

@akraxx
Copy link

akraxx commented May 31, 2017

Jood job @jplock ! I will check the oauth2 part to see if everything is ok after the migration :)

@jplock
Copy link
Member Author

jplock commented May 31, 2017

@akraxx do you want to checkout this branch to see if its still working? I haven't tested that part.

@akraxx
Copy link

akraxx commented Jun 17, 2017

Done in #83 , sorry had no time to do it before !

Upgrade oauth2 support to work with swagger v3
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 72.314% when pulling de646a9 on jp-swagger-ui3 into f80c7fe on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 72.314% when pulling 166590b on jp-swagger-ui3 into f80c7fe on master.

@rstorey
Copy link

rstorey commented Sep 5, 2017

I'm waiting for this to be merged so I can use it in my project without having to build from a fork and use a local maven repo (difficult with Docker deployments). Any chance of it getting merged into the main line soon?

@jplock
Copy link
Member Author

jplock commented Sep 5, 2017

@rstorey my main hesitation right now is that all of the integration tests are going to fail as soon as I merge this in (Travis doesn't execute those because they require Google Chrome right now). So I either have to update the tests (which I don't have time at the moment to do), or remove them (since they were testing that swagger itself was mostly working, I'm leaning toward just removing them).

@rstorey
Copy link

rstorey commented Sep 5, 2017

Gotcha. My opinion: removing the tests is okay. Since what they're actually testing isn't the code of this project, but its underlying dependency, and if any of the tests were to fail, it would be indicative of a problem that isn't necessarily fixable within this project. Thanks for replying.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 72.314% when pulling 5d1dbf8 on jp-swagger-ui3 into f80c7fe on master.

@jplock
Copy link
Member Author

jplock commented Sep 8, 2017

Ok let's do this

@jplock jplock merged commit 712ce55 into master Sep 8, 2017
@jplock jplock deleted the jp-swagger-ui3 branch September 8, 2017 01:15
@rstorey
Copy link

rstorey commented Sep 8, 2017

Yay! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants