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
[cpp-qt-client] pass QString by const reference instead of by value #16793
[cpp-qt-client] pass QString by const reference instead of by value #16793
Conversation
Thanks for the PR.
so is it a breaking change? |
I would say it is not. Indeed, when trying to connect a signal to a slot with a different signature, it still works at least in the test I made. I have two alternative in mind:
What do you think about it ? |
modules/openapi-generator/src/main/resources/cpp-qt-client/api-header.mustache
Outdated
Show resolved
Hide resolved
I prefer this approach |
de5637b
to
4cb6bc4
Compare
@muttleyxd @jimschubert @wing328 any feedback ? |
restarted failed CI jobs for cpp-qt-client but no luck. can you please take a look when you've time? |
@wing328 The error is quite strange since I am not using jurplel/install-qt-action v4... I tried to add a name to disambiguate. |
please resolve the merge conflicts when you've time. |
@wing328 Sorry I missed the notification. I fixed the Qt installation on Windows. |
@wing328 any chance to get this merged on the next release ? |
up |
Let me review and test again this weekend sorry for the delay |
thanks again for the PR, which has been merged. sorry for the delay getting it merged. |
This PR change the signal signature.
PR checklist
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
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(upcoming 7.1.0 minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)@ravinikam @stkrwork @etherealjoy @muttleyxd