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

Implement #188: only override timezone/offset if explicitly defined timezone/offset #191

Merged
merged 1 commit into from
Oct 23, 2020

Conversation

cowtowncoder
Copy link
Member

As per title, try to reduce scope of #175 using new configuration setting.

@cowtowncoder
Copy link
Member Author

Ok, @kupci, @WolfEkk, does this make sense? Basically default timezone/offset will only be used to override one in date/time value if explicitly set when ObjectMapper built, but not if left as default (or, if default specified as null, i.e. remove override).
As per comment there may be need for further configurability (just need to figure out how); but for now seems like a small step we can take.

@WolfEkk
Copy link

WolfEkk commented Oct 20, 2020

I think this is a good compromise, it leaves the opportunity for use cases like mine and for the most part it preserves the behavior expected from #185.

I only wish I could have contributed the fix myself but I couldn't carve enough time out. This is the first time I reach out to you guys and I'd really like to thank you for the attention and the prompt replies and follow-ups.

I'll stick around, maybe I get the chance to contribute, even if just a little in the near future. Have a great week! Looking forward for the rc2 release :)

@cowtowncoder
Copy link
Member Author

@WolfEkk Sounds good, and your help here is appreciated. Date/time handling is one of trickier areas both due to somewhat complex problem domain, and due to wide set of contributions for this module with maybe sometimes different ideas. Would definitely be great if you have time to dig into code at some point here (or other modules).

@cowtowncoder
Copy link
Member Author

Assuming this is generally accepted, merging.

@cowtowncoder cowtowncoder merged commit df62fd4 into 2.12 Oct 23, 2020
@atharvabirtharia
Copy link

hi please see my code too

@cowtowncoder
Copy link
Member Author

@atharvabirtharia Hmmh?

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.

3 participants