Skip to content

fixing bug of rendering an extra spaces into @FeignClient annotation#3487

Merged
wing328 merged 3 commits intoswagger-api:masterfrom
szantopeter:master
Aug 1, 2016
Merged

fixing bug of rendering an extra spaces into @FeignClient annotation#3487
wing328 merged 3 commits intoswagger-api:masterfrom
szantopeter:master

Conversation

@szantopeter
Copy link
Copy Markdown
Contributor

@szantopeter szantopeter commented Jul 31, 2016

FeignClient annotations are redered incorrectly with an extra space. This causes a problem that spring properties are not loaded. E.g: the below annotation was generated

@FeignClient(name="${ swaggerPetstoreSimple.name:swaggerPetstoreSimple}", url="${ swaggerPetstoreSimple.url:https://localhost:8080/api}", configuration = ClientConfiguration.class)

and overwriting

swaggerPetstoreSimple.name:swaggerPetstoreSimple

was not possible because of the extra spaces.

@cbornet
Copy link
Copy Markdown
Contributor

cbornet commented Jul 31, 2016

Can you also regenerate the spring-cloud sample ?

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 1, 2016

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@szantopeter
Copy link
Copy Markdown
Contributor Author

@wing328 thanks for spotting, I fixed my profile so commits should be linked to it
@cbornet I updated the spring/feign examples and added instructions how to generate them (it took me a while to find the shell scripts)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 1, 2016

@szantopeter thanks for the feedback. We've documented Petstore sample update procedure here: https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md#testing but seems like not many people notice it.

We'll probably create a PR template to draw people's attention about updating Petstore sample so that CIs can verify the change.

@wing328 wing328 merged commit 1886abb into swagger-api:master Aug 1, 2016
@szantopeter
Copy link
Copy Markdown
Contributor Author

@wing328 I think most people (users of SWAGGER) will not be as much interested in how to generate the samples, rather what options they need to specify to generate a client from their SWAGGER files using the same technologies. In some cases this is not so obvious because it is not just the language has to be set but also additional options, libraries, etc. Maybe it would worth adding this information to all samples (I only added it to two of them) By the way I read the how to contribute page, but it seems I missed the relevant part at the bottom... ;)

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.

3 participants