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

[Spring] Do not set contextPath for spring-boot #104

Merged
merged 2 commits into from
May 28, 2018

Conversation

cbornet
Copy link
Member

@cbornet cbornet commented May 18, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01)

This fixes swagger-api/swagger-codegen#5244

@cbornet cbornet changed the title [Spring Do not set contextPath for spring-boot [Spring] Do not set contextPath for spring-boot May 18, 2018
@wing328 wing328 added this to the 3.0.0 milestone May 24, 2018
@jmini
Copy link
Member

jmini commented May 24, 2018

Shippable — Run 367 status is FAILED.

The new check to ensure that samples/ is up to date is telling that there are uncommitted changes:

UNCOMMITTED CHANGES ERROR
There are uncommitted changes in working tree after execution of 'bin/ensure-up-to-date'
HEAD detached at FETCH_HEAD
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/AnotherFakeApiController.java
	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/FakeApiController.java
	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/FakeClassnameTestApiController.java
	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/PetApiController.java
	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/StoreApiController.java
	modified:   samples/server/petstore/springboot-reactive/src/main/java/org/openapitools/api/UserApiController.java

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	samples/server/petstore/spring-mvc-j8-async/src/main/resources/openapi.properties
	samples/server/petstore/spring-mvc-j8-localdatetime/src/main/resources/openapi.properties
	samples/server/petstore/spring-mvc/src/main/resources/openapi.propertie

See #80 for discussion. I hope this is not a false positive.

@wing328
Copy link
Member

wing328 commented May 24, 2018

@jmini I simply used the Github web-based GUI to resolve the merge conflicts (to keep both import statements).

I didn't touch those properties files though.

@jmini
Copy link
Member

jmini commented May 24, 2018

The check is also failing on master I have opened #147 to demonstrate the issue and to fix it. When this is merge, maybe you could merge master into this PR.

@wing328
Copy link
Member

wing328 commented May 28, 2018

I'll update the samples in a separate PR.

@wing328 wing328 merged commit 71b5de3 into OpenAPITools:master May 28, 2018
@wing328
Copy link
Member

wing328 commented May 28, 2018

I've run ./bin/spring-all-petstore.sh but git diff doesn't show any change.

The following files are not added to the project:

samples/server/petstore/spring-mvc-j8-async/src/main/resources/openapi.properties
samples/server/petstore/spring-mvc-j8-localdatetime/src/main/resources/openapi.properties
samples/server/petstore/spring-mvc/src/main/resources/openapi.properties

I'll include these files in my next PR.

@cbornet cbornet deleted the spring-basepath branch May 28, 2018 08:58
@jmini
Copy link
Member

jmini commented May 28, 2018

I am not saying that my bin/ensure-up-to-date script introduced by #80 is absolutely correct it might be improved.

But merging a PR with a failed check is a bad idea. Now all PR integrating this commit are failing.

@wing328:
Having a red CI is really annoying for other contributor, other PRs!

I also did not manage to reproduce this locally... In case of emergency like this, we can also remove the spring scripts from the list of checked generators (remove this line:

./bin/spring-all-pestore.sh
)

Can you please have a look?

@jmini
Copy link
Member

jmini commented May 28, 2018

I'll include these files in my next PR.

The missing files are added with #165

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.

basePath specified in swagger file is ignored by spring code generator
3 participants