Move bash argv opt to end of ags line in scripts#124
Merged
wing328 merged 1 commit intoOpenAPITools:masterfrom May 22, 2018
Merged
Move bash argv opt to end of ags line in scripts#124wing328 merged 1 commit intoOpenAPITools:masterfrom
wing328 merged 1 commit intoOpenAPITools:masterfrom
Conversation
The $@ option in bash doesn't make sense to come before `generate` because the only option we can pass before generate cli usage is `help`. System properties can be passed via JAVA_OPTS, so there's not really a need for any intermediaries in the command line construction. Having $@ at the end of the arguments list allows maintainers and users inspecting options to quickly pass new options to a script. For example, ``` ./bin/aspnetcore-petstore.sh --additional-properties sourceFolder=asdf ``` For command line arguments that may appear more than once in the arguments list, this change doesn't provide any rules about overwriting values that may exist (hard-coded) in the script. That is, in the example above, if aspnetcore-petstore.sh already includes the sourceFolder set to a different value, the "winning" value is up to the options parser and openapi-generator-cli implementation.
Member
Author
|
This is a lot of files changed, but basically the same change in almost every file (some java/jaxrs files have multiline I used a recorded macro to make these changes, then verified I had made no mistakes with variations of git-grep on staged files: I don't know if this helps in evaluation, but I assume you could pull the branch and target the HEAD commit with git grep to double-check. |
jmini
approved these changes
May 22, 2018
Member
|
Looks good to me. |
wing328
approved these changes
May 22, 2018
Member
wing328
left a comment
There was a problem hiding this comment.
Thanks for the PR to update all the scripts. (I only updated them one by one when I'm testing the generator)
4 tasks
jmini
reviewed
May 22, 2018
| # if you've executed sbt assembly previously it will use that instead. | ||
| export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties" | ||
| ags="$@ generate -t modules/openapi-generator/src/main/resources/JavaJaxRS/cxf-cdi -i modules/openapi-generator/src/test/resources/2_0/petstore.yaml -l jaxrs-cxf-cdi -o samples/server/petstore/jaxrs-cxf-cdi -DhideGenerationTimestamp=true" | ||
| ="generate -t modules/openapi-generator/src/main/resources/JavaJaxRS/cxf-cdi -i modules/openapi-generator/src/test/resources/2_0/petstore.yaml -l jaxrs-cxf-cdi -o samples/server/petstore/jaxrs-cxf-cdi -DhideGenerationTimestamp=true $@" |
Member
There was a problem hiding this comment.
ags is missing. The line should be ags="generate -t modules/...
Member
|
This PR breaks some script. I found the first one @jimschubert can you have a look and submit an other PR to fix the scripts? |
This was referenced May 22, 2018
jmini
added a commit
to jmini/openapi-generator
that referenced
this pull request
May 22, 2018
jimschubert
pushed a commit
that referenced
this pull request
May 22, 2018
* jaxrs-cxf-cdi: fix model enum * Fix script 'bin/jaxrs-cxf-cdi-petstore-server.sh' See #124 * Run 'bin/jaxrs-cxf-cdi-petstore-server.sh'
jimschubert
added a commit
to jimschubert/openapi-generator
that referenced
this pull request
May 23, 2018
* master: (36 commits) jaxrs-cxf-cdi: fix outer enum (OpenAPITools#131) Move bash argv opt to end of ags line in scripts (OpenAPITools#124) Reduce CI logging (OpenAPITools#119) Download elm dependencies without prompting user. (OpenAPITools#118) [aspnetcore] Make the use of Swashbuckle optional (OpenAPITools#110) DefaultGenerator: fix NullPointerException (OpenAPITools#109) [Java] use html entities in javadoc of generated code (OpenAPITools#106) Update PULL_REQUEST_TEMPLATE.md [java] Enum in array of array (OpenAPITools#66) Rename datatype to dataType in CodegenProperty (OpenAPITools#69) update elm test to compile all elm files (OpenAPITools#95) Fix Petstore example for Elm (OpenAPITools#96) Update Docker documentation (OpenAPITools#97) CaseFormatLambda has been added, params for Rest-assured client has been refactored (OpenAPITools#91) Update integration.md [Clojure] Add util method to set the api-context globally (OpenAPITools#93) [JaxRS-Java] issue with implFolder on windows, and required fields generation for containers (OpenAPITools#88) Set parameters allowableValues dynamically (OpenAPITools#65) Meta: set version for "build-helper-maven-plugin" (OpenAPITools#89) Fix javadoc issues in "openapi-generator" module (OpenAPITools#84) ...
Member
Author
nilskuhn
pushed a commit
to nilskuhn/openapi-generator
that referenced
this pull request
Apr 6, 2023
…est-0.x chore(deps): update dependency type-fest to v0.19.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
see #113
The $@ option in bash doesn't make sense to come before
generatebecause the only option we can pass before generate cli usage is
help.System properties can be passed via JAVA_OPTS, so there's not really a
need for any intermediaries in the command line construction.
Having $@ at the end of the arguments list allows maintainers and users
inspecting options to quickly pass new options to a script. For example,
For command line arguments that may appear more than once in the
arguments list, this change doesn't provide any rules about overwriting
values that may exist (hard-coded) in the script. That is, in the
example above, if aspnetcore-petstore.sh already includes the
sourceFolder set to a different value, the "winning" value is up to the
options parser and openapi-generator-cli implementation.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif 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\.master.