Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Jaxrs] Add beanvalidation annotations and fix outer Enums #4492

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

Merged
wing328 merged 9 commits into swagger-api:master from jfiala:jaxrs_beanval
Jan 19, 2017

Conversation

@jfiala
Copy link
Contributor

@jfiala jfiala commented Jan 4, 2017
edited
Loading

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

@wing328 wing328 Jan 9, 2017

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.

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.

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?

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).

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 👍

Copy link
Contributor

wing328 commented Jan 12, 2017

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?

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
Copy link
Contributor

wing328 commented Jan 19, 2017

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

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
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.

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

Reviewers

1 more reviewer

@wing328 wing328 wing328 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

v2.2.2

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /