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.

Sign up for GitHub

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

Jump to bottom

[JAX-RS] Show correct default value on CLI option description #3862

Merged
merged 2 commits into from
Sep 24, 2016
Merged

[JAX-RS] Show correct default value on CLI option description #3862

merged 2 commits into from
Sep 24, 2016

Conversation

y-higuchi
Copy link
Contributor

@y-higuchi y-higuchi commented Sep 24, 2016 •
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 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

Fixed jaxrs cli option handling to print correct default library template.

Fixes #3861

@y-higuchi
Copy link
Contributor Author

After running ./bin/jaxrs-petstore-server.sh

I've notices ^M in generated code, and it seems to be caused by modules/swagger-codegen/src/main/resources/JavaJaxRS/api.mustache.

Is it OK for it to be CRLF? or should api.mustache be fixed to LF

@wing328
Copy link
Contributor

wing328 commented Sep 24, 2016

@y-higuchi I did a search for ^M (ctrl V + ctrl M in vim) but couldn't find it in modules/swagger-codegen/src/main/resources/JavaJaxRS/api.mustache.

Can you tell me which line(s) you found ^M?

@wing328
Copy link
Contributor

wing328 commented Sep 24, 2016

(btw, I'm using latest master to check so please pull the latest master if you've not done so)

@wing328 wing328 added this to the v2.2.2 milestone Sep 24, 2016
@y-higuchi
Copy link
Contributor Author

I'm on fedb0cc so I think I'm on the latest master, but I am getting:

$ file modules/swagger-codegen/src/main/resources/JavaJaxRS/api.mustache
modules/swagger-codegen/src/main/resources/JavaJaxRS/api.mustache: ASCII Java program text, with CRLF line terminators

Looking with Atom editor, it seems all lines in that file is CRLF

@wing328
Copy link
Contributor

wing328 commented Sep 24, 2016

OK. I'll take another look using another editor.

Your change looks good. Thanks for the contribution :)

@wing328 wing328 merged commit 2155b7b into swagger-api:master Sep 24, 2016
@y-higuchi y-higuchi deleted the jaxrs_default_library branch September 24, 2016 07:11
@wing328
Copy link
Contributor

wing328 commented Sep 24, 2016

I'm able to locate those ^M and will file a PR to clean those up this weekend.

I'll also do a grep on all auto-generated code to ensure there's no ^M.

Thanks for spotting the issue.

@wing328
Copy link
Contributor

wing328 commented Sep 24, 2016

@y-higuchi UPDATE: I've filed #3865 to fix the line breaks.

acramatte added a commit to comerge/swagger-codegen that referenced this pull request Oct 4, 2016
@acramatte
Merge remote-tracking branch 'upstream/master'
cec9d47 
* upstream/master: (79 commits)
  add undertow
  Add a new cli command to output version information (2nd attempt) swagger-api#3892 (swagger-api#3899)
  fix python flask controller without tag (default_controller)
  [aspnet5] Fix basePath application to operations (swagger-api#3911)
  Bugfix/issue 3723 (swagger-api#3726)
  Cgardens nested object regex (swagger-api#3879)
  [Cpprest] Fixing issue swagger-api#3773 (swagger-api#3876)
  escape callback parameter for java(okhttp) and python
  fix warning in html generator
  [PHP] fix PHPUnit invocation, add basic phpunit.xml.dist (swagger-api#3864)
  [Java] Remove duplicated model description in Spring, JAX-RS models (swagger-api#3887)
  [PHP] Better PSR2 compatibility (swagger-api#3863)
  Mention security script in pull request template
  [Swift] Use thread safe manager dictionary
  Replace ^M with new line (\r) in mustache template (swagger-api#3865)
  [swfit] fix url param with base name
  [JaxRS]Show correct default value on CLI option description (swagger-api#3862)
  add title, description to HTML output (swagger-api#3860)
  fix trailing comma in go api client
  fix typescript-fetch base path by removing ending slash
  ...
@wing328
Copy link
Contributor

wing328 commented Jan 9, 2017

FYI. #4526 to detect the issue moving forward.

@wing328 wing328 changed the title [JaxRS]Show correct default value on CLI option description [JAX-RS] Show correct default value on CLI option description Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Server: Java
Projects
None yet
Milestone
v2.2.2
Development

Successfully merging this pull request may close these issues.
2 participants
@y-higuchi @wing328