-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Update JOptions configuration (Lombiq Technologies: OCORE-149) #15534
Conversation
This fix is good and will result in one less breaking change. I am not sure if there is a negative impact from this change. @sebastienros any thoughts if there is a downside from taking this change? |
@sarahelsaig on a second note, I think this change should not be part of Base (maybe we do, but probably not). It should only be part of the ContentSerializerOptions Not that ContentSerializerOptions is the only options that impact serializing and deserializing object in YesSql. |
Won't it be bad if Base has a different ReferenceHandler than ContentSerializerOptions? |
I don't think so. YesSQL can has different options. for example, in the YesSQL we add converters for different types. |
@MikeAlhayek I've made the change you suggested and unfortunately it won't work. I don't think the problem is only related to YesSQL. I guess your approach works as long as the original structure (i.e. the So I think this config won't be usable unless we put it into |
Fixes #15533