Skip to content

Conversation

@huang195
Copy link

@huang195 huang195 commented Mar 7, 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: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This is to resolve the issue described in kubernetes-client/python#471.
From my PR kubernetes-client/python#473, it was suggested that I should open a PR here instead.

@wing328
Copy link
Contributor

wing328 commented Mar 8, 2018

@huang195 thanks for the PR.

cc @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) to review.

There are issues with the Python3 tests in Travis. It seems like there are changes to the Travis image causing the errors. I'll look into that tomorrow.

@huang195
Copy link
Author

huang195 commented Mar 8, 2018

@wing328 ok thanks. It looks like the Travis error in the PR is due to the change to this file: samples/client/petstore/python/docs/EnumTest.md. This was autogenerated by running bin/python-petstore.sh.

Initially, I thought this was changed due to me adding an additional parameter to a class constructor, but running bin/python-petstore.sh directly on the head of the master branch would also change samples/client/petstore/python/docs/EnumTest.md. Would appreciate any pointer if there's something I missed on my end.

@wing328
Copy link
Contributor

wing328 commented Mar 25, 2018

@huang195 likely I'll need to comment out the enum_string_required test as it's not ready for all generators yet. I'll do it in the coming week and keep you posted.

Thanks again for the contribution.

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.

3 participants