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

#1983 Allow case insensitive properties and values #2619

Merged
merged 2 commits into from
Feb 17, 2020

Conversation

alevskyi
Copy link
Contributor

@alevskyi alevskyi commented Feb 7, 2020

Fixes #1983.

@cowtowncoder
Copy link
Member

That was quick! Thank you; I hope to review this soon.

}

public void testReadMixedCaseSubclass() throws IOException {
// There is also "ACCEPT_CASE_INSENSITIVE_VALUES" feature. Is it more suitable here, or I should leave it as is?
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm. Good question. I will have to think about this -- VALUES seems slightly better. But I will need to think through consequences and see what existing documentation says.

@cowtowncoder
Copy link
Member

Quick note: same as with jackson-core contribution, just need that one CLA (no separate ones needed, covers all Jackson projects, contributions) and can then merge this.

@cowtowncoder
Copy link
Member

Also: I just realized that both features are relevant: one to ensure that virtual property name is compared in case-insensitive manner; and the other that value (type id) is -- so I think your suggestion is correct wrt which one to use and where.

@cowtowncoder cowtowncoder merged commit e5d22e2 into FasterXML:2.11 Feb 17, 2020
@cowtowncoder cowtowncoder modified the milestones: 1.9.13, 2.11.0 Feb 17, 2020
@cowtowncoder
Copy link
Member

@alevskyi Thank you again for the fix! I made some minor tweaks (to match in all cases, I hope), will be in 2.11.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants