Skip to content

Conversation

@JooHyukKim
Copy link
Member

No description provided.

@cowtowncoder
Copy link
Member

@JooHyukKim This seems incomplete? Or maybe diff is missing something due to conflict?

@JooHyukKim
Copy link
Member Author

@cowtowncoder Yes, there were some other commits on same file, affecting this PR. THankss

@JooHyukKim
Copy link
Member Author

Ready for review! Just in case I wasn't clear enough

*/
protected final boolean _isShapeNumberInt;

public EnumDeserializer(EnumResolver byNameResolver, boolean caseInsensitive,
Copy link
Member

Choose a reason for hiding this comment

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

Need to start deprecating methods, adding overloads (since it's 3.1, not 3.0)

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

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

LGTM

for (int i = enumConstants.length; --i >= 0; ) { // from last to first, so that in case of duplicate values, first wins
Enum<?> en = enumConstants[i];
try {
Object o = accessor.getValue(en);
Copy link
Member

Choose a reason for hiding this comment

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

What is being accessed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Value returned from method annotated with @JsonValue.
What could be a good improvement here?

Copy link
Member

Choose a reason for hiding this comment

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

Hmmh. Maybe explain calling @JsonValue annotated accessor (Field/Method) on every enum entry?

@JooHyukKim
Copy link
Member Author

Thank you for the review @cowtowncoder !
Applied suggestions, and wrote just one question on this comment

@JooHyukKim JooHyukKim closed this Nov 2, 2025
@JooHyukKim JooHyukKim reopened this Nov 2, 2025
@JooHyukKim
Copy link
Member Author

I re-examined paths before adding more JavaDoc, it just works without any additional implementation 🤔.
Will check if 2.x is same.

@JooHyukKim
Copy link
Member Author

Closing in favor of #5376

@JooHyukKim JooHyukKim closed this Nov 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants