search

FasterXML / jackson-datatype-joda

Extension module to properly support full datatype set of Joda datetime library
Apache License 2.0
140 stars 81 forks source link

Change `IntervalDeserializer` to respect input timezone when enabled #105

Closed Woodz closed 5 years ago

Woodz commented 6 years ago

Bump dependency on joda-time to v2.9 to access Interval.parseWithOffset Change IntervalDeserializer to use Interval.parseWithOffset and override timezone only if shouldAdjustToContextTimeZone is set or explicit timezone is specified in the format Add unit tests for IntervalDeserializer to cover the 3 scenarios:

Woodz commented 5 years ago

@cowtowncoder any chance you can review and merge this PR?

cowtowncoder commented 5 years ago

@Woodz sorry, this slipped through the cracks.

cowtowncoder commented 5 years ago

Hmmh. While I otherwise like the change, requirement to use Joda 2.9 needs some thought. I realize it has been around for 4 years now, but upgrade is still a breaking change for some users.

Jackson 2.10 does actually use 2.9.9 as build dependency, but should work with older versions.

So this needs to go in a new minor version: I think 2.11 is fine (2.10.0 just went out, for better or worse).

With that, I'd be almost ready to merge. Just one more thing: unless I have asked for it already, CLA would be needed from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and the easiest way is to print, fill, sign & scan, email to info at fasterxml dot com. With that, I can proceed with the new branch, merging.

Thank you again for contributing this, apologies for slow follow up.

cowtowncoder commented 5 years ago

Sent a note to jackson-dev: unless anything drastic heard, planning on merging this in 2.11 branch. Would be great to rebase, but even if not I can just manually patch as the change is nicely compact.

Woodz commented 5 years ago

@cowtowncoder I have emailed over the CLA as requested and have rebased this commit onto master to go into the 2.11 release. Unfortunately I no longer have access to the previous branch (I have since left the organisation under which I created it), so forked a new repo under my personal account and created a PR #109 to replace this one. Apologies for the inconvenience of changing PRs

cowtowncoder commented 5 years ago

@Woodz thank you for the new PR -- not a problem at all, and makes my life easier. Also thank you for sending CLA so quickly.