OpenAPITools
Update HttpRequest.cpp.mustache - use stable 4-parameter connect
The 3-parameter connect is known to cause issues when the creating instance is deleted. In all other mustache you already use stable 4-parameter connect. In HttpRequest.cpp.mustache the fix was missing.
PR checklist
- [x] Read the contribution guidelines.
- [x] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
- [x] Run the following to build the project and update samples:
(For Windows users, please run the script in Git BASH) Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example./mvnw clean package ./bin/generate-samples.sh ./bin/configs/*.yaml ./bin/utils/export_docs_generators.sh./bin/generate-samples.sh bin/configs/java*. IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed. - [x] File the PR against the correct branch:
master(upcoming 7.6.0 minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks) - [x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.
thanks for the PR. please follow step 3 to update the samples.
cc @ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @martindelille (2018/03) @muttleyxd (2019/08)
I ran the parts of step 3, committed and pushed. Put I don't see a hint that these changes came in. Any hint what might went wrong?
Reviewing the changes I see a lot of path separator changes, obviously because I ran mvn and the scripts on Windows. Is that really what you want?
@muttleyxd @MartinDelille when you guys have time, can you please PM me via Slack? Thanks.
Yes please push only the change regarding HttpRequest please!
As far as I see there is nothing changed but the manual change in HttpRequest, as long as the shifted entries of ApiException.php and readme.MD are irrelevant.
From a Windows user perspective the script I was asked to follow in step 3 is unusable in this state. Maybe it should be extended to replace all CR/LF by single LF.
I don't have the time to dig into it but the git bash seems to have commands for this:
find . -name "*.java" -exec dos2unix {} \;
@Jazzco tests with updated samples failed: https://github.com/OpenAPITools/openapi-generator/actions/runs/9527512386/job/26264196606?pr=18939
can you please review the test failure when you've time?
@Jazzco tests with updated samples failed: https://github.com/OpenAPITools/openapi-generator/actions/runs/9527512386/job/26264196606?pr=18939
can you please review the test failure when you've time?
@wing328 I reviewed the change and don't see what could cause the issue. As mentioned above, step 3 produces many line break changes when processed on Windows, so I reverted it. Maybe I oversaw a generated change that was necessary but I guess it makes way more sense to run this script on Linux in the current state.
Locally generating our api and patching the results with these two changes afterwards works fine.
@ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @martindelille (2018/03) @muttleyxd (2019/08) Could one of you please take over this tiny change? And maybe trigger your scripting team to fix the linefeed-issue
Regards Jazzco
@muttleyxd @MartinDelille when you guys have time, can you please PM me via Slack? Thanks.
I sent you an email!
I noticed that there were some other [clazy-connect-3arg-lambda] warnings, this time in api-body, and fixed them, too.
To get this right I ran step 3 from a Linux vm and pushed the generated changes now.
I tested these changes using Qt 5.15.2 on Windows. There were no compilation or link errors.
There were () missing and it obviously worked with the 3 param version but failed with the 4 param version under macOS. Hopefully it builds now.
the CI tests failed: https://github.com/OpenAPITools/openapi-generator/actions/runs/9567139672/job/26402928612?pr=18893
please take a look when you've time.
I'm out of ideas. The linker error is way to universal to track it down to a cause.
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Maybe if you were able to activate the -v option...
Maybe if you were able to activate the -v option...
please update the workflow file to activate that
.github/workflows/samples-cpp-qt-client.yaml
I tried to reproduce the issue locally on my mac using Qt 5.15.17 without issue.
triggered the workflow but some tests failed. please review when you guys have time
I create a draft PR to troubleshoot if the problem comes from the Qt installation in the CI: #19049
The issue is not related to this PR changed. It should be fixed after #19049 is merged.
@wing328 not sure the CI was triggered properly. Can you restart it by hand ?
https://github.com/OpenAPITools/openapi-generator/actions/runs/9954301257/job/27620131137?pr=18893
please update the samples one more time
@Jazzco can you make sure you have regenerated the sample accordingly ?
@Jazzco can you make sure you have regenerated the sample accordingly ?
done
@wing328 It looks like the CI wasnt' triggered properly. Any idea why ?
There was a timeout warning. Maybe some resources were busy. I merged master in again to trigger a rebuild.