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

[C][Client] Remove broken intToStr() macro, update samples #20383

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

imaami
Copy link
Contributor

@imaami imaami commented Dec 27, 2024

Note: this does not fix anything else; for example all numerical types, including float and double, are still incorrectly cast to to long int before string conversion.

@eafer
Copy link
Contributor

eafer commented Dec 28, 2024

@imaami imaami force-pushed the fix-c-int-to-string branch from efd2ba6 to 6608bcd Compare January 1, 2025 00:45
@imaami
Copy link
Contributor Author

imaami commented Jan 1, 2025

Rebased onto current master, made sure to also update the samples just in case something changed.

@eafer
Copy link
Contributor

eafer commented Jan 1, 2025

Hey @imaami , I sent you a review comment around a week ago. I think you can't see it for some reason? I'm always confused with github. Anyway, this was the comment:

- localVarPath = strReplace(localVarPath, localVarToReplace_{{paramName}}, localVarBuff_{{paramName}});

What's going on here? Is the call to strReplace() not needed?

@imaami
Copy link
Contributor Author

imaami commented Jan 1, 2025

Hey @imaami , I sent you a review comment around a week ago. I think you can't see it for some reason? I'm always confused with github. Anyway, this was the comment:

- localVarPath = strReplace(localVarPath, localVarToReplace_{{paramName}}, localVarBuff_{{paramName}});
What's going on here? Is the call to strReplace() not needed?

Oh damn! I somehow missed it. Let me look at that now...

@imaami
Copy link
Contributor Author

imaami commented Jan 1, 2025

@eafer I still don't know where that comment might be, but good catch! Completely a screw-up on my part, the strReplace() call should not be removed. I'll update the commit and regen the samples. (I don't think the samples will actually change because isFloat isn't active in any samples as far as I can see.)

Note: this does not fix anything else; for example all numerical
types, including float and double, are still incorrectly cast to
to long int before string conversion.
@imaami imaami force-pushed the fix-c-int-to-string branch from 6608bcd to 044141e Compare January 1, 2025 22:34
@imaami
Copy link
Contributor Author

imaami commented Jan 1, 2025

@eafer OK, fixed, rebased onto current master, and regenerated the samples (with no change in the result).

Copy link
Contributor

@eafer eafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now.

@wing328 wing328 added this to the 7.11.0 milestone Jan 2, 2025
@wing328 wing328 merged commit 43f59ba into OpenAPITools:master Jan 2, 2025
4 checks passed
@imaami imaami deleted the fix-c-int-to-string branch January 2, 2025 15:37
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