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

Try to merge #4271 (for #1467) to master (2 failures wrt Creator name mismatch) #4797

Merged
merged 9 commits into from
Nov 14, 2024

Conversation

cowtowncoder
Copy link
Member

So: hoping to get #4271 merged, but this gets tricky for one specific aspect.
Hoping to get it resolved, otherwise cannot get feature in 3.0.

@cowtowncoder
Copy link
Member Author

cowtowncoder commented Nov 13, 2024

Hmmmh. So close yet so far. Was able to figure out issue with renameAll -- its call had been relocated for 3.0 earlier, so BeanDeserializerBase should not need to try to call it.
And of 4 test failures, 2 were for obsolete (but remaining in 3.0, due to same merge issue), wrt #265, so was able to just remove test class that contains it.

But this leaves 2 remaining test failures that seem legit.
I wonder if you might be able to help see if you spot anything @fxshlein ? I know 3.0 (master branch) is quite changed from 2.x and things are moved around.
But any help would be appreciated; I'd really want this feature to work for 3.0 as well as 2.x and we are close :)

EDIT: exception message seems to suggest mismatch between incoming names, creator parameters (which is why "fallback setter" is being checked), so I am suspecting some missing renaming (or missing re-creation of matchers).

cc @JooHyukKim

@JooHyukKim
Copy link
Member

Seems like a big work we got here :-)
Will try to see what we can do about it, probably tonight!

@cowtowncoder
Copy link
Member Author

@JooHyukKim thanks! I'm sure all of us together can figure it out. In the worst case could just move 2 failing tests under "tofix".

@cowtowncoder cowtowncoder changed the title Try to merge #4271 (for #1467) to master; problem with lack of access to DeserializationContext Try to merge #4271 (for #1467) to master (2 failures wrt Creator name mismatch) Nov 13, 2024
@cowtowncoder cowtowncoder merged commit c6c4007 into master Nov 14, 2024
6 checks passed
@cowtowncoder cowtowncoder deleted the tatu/3.0/4271-merge-jsonunwrapped-via-creator branch November 14, 2024 04:59
@cowtowncoder
Copy link
Member Author

Decided to move 2 failing tests under tofix; can figure things out in future.

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