Skip to content

Conversation

@jfiala
Copy link
Contributor

@jfiala jfiala commented Jan 4, 2017

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch 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)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

{
public class JavaJAXRSSpecServerCodegen extends AbstractJavaJAXRSServerCodegen implements BeanValidationFeatures
{
protected boolean useBeanValidation = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we keep useBeanValidation to false similar to what we've done for other Java-related generator: https://github.com/swagger-api/swagger-codegen/search?l=Java&q=useBeanValidation&utf8=%E2%9C%93

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as #4506, please advise if the decision is to keep the default to false (current impl of cxf/java) or true (suggestion of @cbornet)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jfiala I'll think about this more and get back to you.

@wing328
Copy link
Contributor

wing328 commented Jan 12, 2017

@jfiala the Travis CI failed with the following error message:

The command "set -e" exited with 0.
$ /bin/bash ./bin/utils/detect_carriage_return.sh
modules/swagger-codegen/src/main/resources/JavaJaxRS/spec/generatedAnnotation.mustache
modules/swagger-codegen/src/main/resources/JavaJaxRS/spec/beanValidation.mustache
Templates contain carriage return '/r'. Please remove it and try again.

Please take a look.

@jfiala
Copy link
Contributor Author

jfiala commented Jan 12, 2017

@wing328 I changed my git settings (on Windows) to git config --global core.autocrlf input and then did a reset for the branch as described here (https://help.github.com/articles/dealing-with-line-endings/#refreshing-a-repository-after-changing-line-endings) and checked in with corrected lf settings.
Is the recommended setting for autocrlf documented somewhere?

@jfiala
Copy link
Contributor Author

jfiala commented Jan 12, 2017

@wing328 the crlf are fixed now (one build is still failing due to an error at the Java library ok-http-gson, but I didn't change anything there).

@wing328
Copy link
Contributor

wing328 commented Jan 12, 2017

Is the recommended setting for autocrlf documented somewhere?

It's not documented in our wiki at the moment. I'll put it in the contributing guideline shortlly.

Thanks for the suggestion 👍

@wing328
Copy link
Contributor

wing328 commented Jan 12, 2017

@jfiala
Copy link
Contributor Author

jfiala commented Jan 17, 2017

@wing328 is there already an ETA for the next release 2.2.2?
I'd like to further work on the Jaxrs-languages (interface etc.), but I need my Beanvalidation-PRs to first get pulled in.
Should I add additional branches starting from the jaxrs-branches in my forked repo? Or is it better to wait for my PRs to get pulled in so I can restart from the (upstream) master?

@wing328
Copy link
Contributor

wing328 commented Jan 18, 2017

@jfiala let me try to review later today and merge this in if there's no feedback from me or anyone from the community.

ETA for 2.2.2 is Jan/Feb 2017

@wing328 wing328 added this to the v2.2.2 milestone Jan 18, 2017
@wing328 wing328 merged commit ee7f9fc into swagger-api:master Jan 19, 2017
@wing328
Copy link
Contributor

wing328 commented Jan 19, 2017

@jfiala I ran some tests and didn't find any issue. Thanks for the enhancements.

@jfiala
Copy link
Contributor Author

jfiala commented Jan 19, 2017

@wing328 great, thx can you pls also take a look at the other beanvalidation PR's so we get them all in?

@jfiala jfiala deleted the jaxrs_beanval branch January 19, 2017 08:20
@wing328
Copy link
Contributor

wing328 commented Jan 19, 2017

@jfiala yup, merged a few already. Would need your help to rebase one of them on the latest master.

@jfiala
Copy link
Contributor Author

jfiala commented Jan 19, 2017

@wing328 thx alot, I already noticed, I'll rebase the spring-branch and then let you know.

davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
…pi#4492)

* add beanvalidation to jaxrs and add support for outer Enums swagger-api#4091

* cleanup Codegen swagger-api#4091

* cleanup samples swagger-api#4091

* cleanup tabs

* updated samples to petstore.yaml (before petstore.json)

* add support for DecimalMin/DecimalMax/Min/Max swagger-api#4091

* add check for hideGenerationTimestamp swagger-api#4091

* replace tabs

* correct line endings to lf
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.

2 participants