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.

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

Fix export_generator shell script on Linux #1223

Merged
merged 3 commits into from
Oct 14, 2018

Conversation

fujigon
Copy link
Contributor

@fujigon fujigon commented Oct 12, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell 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). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

./bin/utils/export_docs_generators.sh (and its depending ./bin/utils/export_generators.sh) has string pattern bug.

@fujigon
Copy link
Contributor Author

fujigon commented Oct 12, 2018

re-run ./bin/utils/export_docs_generators.sh and committed all docs/generators/*.md

@fujigon fujigon changed the title [WIP] fix export_generator shell script fix export_generator shell script Oct 12, 2018
@fujigon
Copy link
Contributor Author

fujigon commented Oct 12, 2018

@wing328 wing328 added this to the 3.3.1 milestone Oct 12, 2018
@wing328
Copy link
Member

wing328 commented Oct 12, 2018

@fujigon thanks for the PR. Reviewing...

@wing328
Copy link
Member

wing328 commented Oct 14, 2018

@fujigon I tried to trigger a build failure via 8d82fc6 but the build still passed: https://circleci.com/gh/OpenAPITools/openapi-generator/2853. Still investigating why...

@wing328
Copy link
Member

wing328 commented Oct 14, 2018

I've a fix and https://circleci.com/gh/OpenAPITools/openapi-generator/2870#tests/containers/2 reported failure, which is the expected behaviour.

@wing328
Copy link
Member

wing328 commented Oct 14, 2018

@fujigon I've pushed the fix to your branch and the circle CI build is good (shippable failure is not related to this PR). Merging this into master. Thanks again for the fix.

cc @jmini

@wing328 wing328 merged commit fbd4411 into OpenAPITools:master Oct 14, 2018
@wing328 wing328 changed the title fix export_generator shell script Fix export_generator shell script on Linux Oct 14, 2018
@jimschubert
Copy link
Member

@wing328 @fujigon I was reviewing this, but I couldn't understand how the change in this PR had any difference in sh, since the markdown outputs resulted in the same generator name outputs (only difference was the added generator properties). I think the bash syntax could be cleaned up a little more, but I wanted to understand the error that was being solved. Was the problem perhaps a sed portability issue?

I was testing on the sed shipped with Mac 10.13.6.

@jimschubert
Copy link
Member

Sorry, I missed the on Linux in the PR title. I opened another PR to move the work being done in sed and output redirection into the config-help task itself.

@fujigon fujigon deleted the oneoff/doc-script branch October 15, 2018 01:48
@fujigon
Copy link
Contributor Author

fujigon commented Oct 15, 2018

thanks for the review and merging!
I confirmed that ./bin/utils/ensure-up-to-date has no git diff about docs on Linux with current master.

@wing328
Copy link
Member

wing328 commented Oct 16, 2018

@fujigon thanks for the fix, which has been included in the v3.3.1 release: https://twitter.com/oas_generator/status/1052020299821080577

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* fix export_generator shell script

* fix script with bash

* use bash instead of sh
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.

None yet

3 participants