-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[Python] Fix delimiter collision issue #5981 #5982
[Python] Fix delimiter collision issue #5981 #5982
Conversation
aargh.. I missed the step to commit the Petstore samples related to my fix. Doing it now.. |
To update the Petstore samples, I ran
Question: Do I need to run
@taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01) @slash-arun (2019/11) @spacether (2019/11) |
Getting a bit more familiar with CircleCI now.. found my answer in the logs. I need to commit all the "uncommitted changes in working tree after execution of 'bin/ensure-up-to-date'". |
Why did you close this? |
Hi @spacether, I actually did |
Why are there java and dart changes in this PR? Should python only be in here? |
I committed all files that were modified after running Also, do I commit only the python I also can't get circle-ci to build my forked repo. My last 2 attempts after syncing with upstream: Hope you can provide clear steps/example that I can follow to complete this PR. Thanks.. |
If there are non-python files that ensure up to date swept up then something is wrong.
|
@spacether Thanks! I'll try your suggestions and get back here. |
@spacether Whatever I do it seems (rebasing or hard reset to upstream/master), I could not get my master branch to not generate ensure up to date changes.
I could delete my fork, recreate it and try ensure-up-to-date again. Deleting my fork will probably close this PR. In any case, I'll have to resolve this in my branch first. |
Yup, it sounds like something is not working with your branch/fork. Have you tried rebasing your forked master on upstream/master? I do:
And you could also do that on this PR branch to base it on the latest upstream master commit. |
@spacether Thanks.. I've tried rebasing my forked master on upstream/master but to be sure I'm trying it again with your steps exactly on this PR branch. Circleci build still failed on my forked master. Will delete and re-fork. |
@spacether still no luck with a new fork. Here is my exact steps. Am I missing something?
Got exactly the same UNCOMMITTED CHANGES ERROR:
|
Can you ask your question in the openapi generator #general slack channel? I do not know what would cause that error but someone there should know what is causing it and how to fix it. The slack link is in the Readme file. |
Sorry for the delay here. So I talked to a core team member and figured this our.
|
@spacether Thank you so much for figuring this out. I'll try out your suggestions when I get the chance this week. |
@spacether I tried the updated steps on a fresh fork,
And
|
Hmm did you rebase on master and then rebuild with |
@fullcircle23 another solution here is to remove or revert your samples update commits and only run:
Usually we depend upon CI to run ensure-up-to-date and you don't need to run it locally because you are only changing one or two samples folders. |
It was from a clone of a new fork so I didn't need to rebase it. So basically I did steps 1 to 6 exactly:
About 5 hours ago. |
@fullcircle23 if you need help on this, please PM me via Slack (https://join.slack.com/t/openapi-generator/shared_invite/enQtNzAyNDMyOTU0OTE1LTY5ZDBiNDI5NzI5ZjQ1Y2E5OWVjMjZkYzY1ZGM2MWQ4YWFjMzcyNDY5MGI4NjQxNDBiMTlmZTc5NjY2ZTQ5MGM) |
Please file a new PR instead as this PR shows "unknown repository" |
Closing this PR, replaced by #6451 |
Wrap the default and example value string literals using double quotes (instead of single quote) to avoid the need to escape any embedded single quotes and also to avoid any invalid syntax errors due to delimiter collision in the generated test code.
Also:
Open Issues
If merged, this will resolve #5981
PR checklist
./bin/
(or Windows batch scripts under.\bin\windows
) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh
,./bin/openapi3/{LANG}-petstore.sh
if updating the code or mustache templates for a language ({LANG}
) (e.g. php, ruby, python, etc).master
,4.3.x
,5.0.x
. Default:master
.@taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01) @slash-arun (2019/11) @spacether (2019/11)