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

Support for short and uncommon field names like set, get, and is #44457

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

burl21
Copy link
Contributor

@burl21 burl21 commented Nov 12, 2024

fix: #44433
Enhance handling of short field names specifically for records, ensuring correct support for fields like set, get, and is.

Copy link

quarkus-bot bot commented Nov 12, 2024

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with dot

This message is automatically generated by a bot.

@burl21 burl21 changed the title Support for short and uncommon field names like set, get, and is. Support for short and uncommon field names like set, get, and is Nov 12, 2024
@gsmet
Copy link
Member

gsmet commented Nov 13, 2024

/cc @mariofusco

@@ -281,7 +281,7 @@ private Type fieldType() {
if (isPublicField()) {
return fieldInfo.type();
}
if (methodInfo.name().startsWith("set")) {
if (methodInfo.parametersCount() == 1 && methodInfo.name().startsWith("set")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@mariofusco mariofusco left a comment

Choose a reason for hiding this comment

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

@burl21 Thanks a lot for this fix, I'd just prefer to keep it more confined to the "set", "get" and "is" field name it is supposed to support, so please see my suggestion.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I rebased and squashed the commits. Let's wait for CI and merge!

Thanks!

Copy link

quarkus-bot bot commented Nov 13, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7bbb0a8.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet gsmet merged commit 7fa4088 into quarkusio:main Nov 13, 2024
32 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 13, 2024
@gsmet
Copy link
Member

gsmet commented Nov 13, 2024

Merged, thanks for your contribution.

The fix will land in 3.16.4 that we will release next week.

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.

Reflection free serializers ArrayIndexOutOfBoundsException
3 participants