Skip to content

JAVA-1161: Preserve full time zone info in ZonedDateTimeCodec and DateTimeCodec#676

Merged
olim7t merged 1 commit into
3.0from
java1161
Jul 15, 2016
Merged

JAVA-1161: Preserve full time zone info in ZonedDateTimeCodec and DateTimeCodec#676
olim7t merged 1 commit into
3.0from
java1161

Conversation

@adutra
Copy link
Copy Markdown
Contributor

@adutra adutra commented May 20, 2016 •
edited
Loading

The present PR proposes to store zone IDs (such as UTC, GMT, or Europe/Paris) instead of raw offsets (such as +00:00 or -07:00), when this information is available.

Zone IDs carry daylight savings time information, which offsets do not, and so can be more precise. One small drawback is that zone IDs are not ISO-8601 normalized, but I guess we can live with that.

I also strived to make both affected codecs, ZonedDateTimeCodec and DateTimeCodec, compatible with each other, i.e., a tuple serialized by the former should be deserializable by the latter.

And finally, I opened ZonedDateTimeCodec to custom formatters, in case users would want to format or parse in a specific format. I did the same for DateTimeCodec but only for the timestamp part, mainly because there is no proper parser/formatter for time zones: they have to be parsed by the built-in parser in DateTimeZone.forID(), which does not leave much room for customization.

@adutra adutra added this to the 3.1.0 milestone May 20, 2016
@adutra adutra changed the title ZonedDateTime should be serialized and formatted with full informatio… ZonedDateTimeCodec and DateTimeCodec do not preserve time zone ID, only the offset May 20, 2016
@adutra adutra force-pushed the java1161 branch 2 times, most recently from 9f05ac9 to 84dc47b Compare May 23, 2016 09:34
@adutra adutra changed the title ZonedDateTimeCodec and DateTimeCodec do not preserve time zone ID, only the offset JAVA-1161: ZonedDateTimeCodec and DateTimeCodec do not preserve time zone ID, only the offset May 23, 2016
@adutra
Copy link
Copy Markdown
Contributor Author

adutra commented Jun 17, 2016

Rebased on top of current 3.0.

@adutra adutra changed the title JAVA-1161: ZonedDateTimeCodec and DateTimeCodec do not preserve time zone ID, only the offset JAVA-1161: Preserve full time zone info in ZonedDateTimeCodec and DateTimeCodec Jul 7, 2016
@adutra
Copy link
Copy Markdown
Contributor Author

adutra commented Jul 7, 2016

Rebased again. Il would like to have the team's opinion about how to handle warnings in IntelliJ when a class uses APIs from Java 7+. The default behavior is to raise a compile warning, which is very useful. These errors can be suppressed with @SuppressWarnings("Since15") – which is why all classes in com.datastax.driver.extras.codecs.jdk8 have this annotation. Unfortunately, this does not apply to imports, so we have to avoid imports and specify the class FQDN each time (example here).
But in hindsight, it makes the whole code quite verbose, so maybe a better approach would be to raise the alert from error to warning or weak warning. Thoughts?

@tolbertam
Copy link
Copy Markdown
Contributor

tolbertam commented Jul 8, 2016

The default behavior is to raise a compile warning, which is very useful. These errors can be suppressed with @SuppressWarnings("Since15") – which is why all classes in com.datastax.driver.extras.codecs.jdk8 have this annotation.

If I remove the FQDN from DateTimeFormatter I don't get the warning, maybe this is an IntellIJ bug that has since been fixed? I'm on version 15.0.6.

@adutra
Copy link
Copy Markdown
Contributor Author

adutra commented Jul 11, 2016

maybe this is an IntellIJ bug that has since been fixed? I'm on version 15.0.6.

I'm on – err – 2016.1.3 (they completely changed their versioning scheme, so I'm a bit lost).

By default, under "Preferences > Editor > Inspections > Java > Java language level migration aids", I had an inspection called "Usages of API which isn't available at the configured language level", set to Error severity.

@tolbertam
Copy link
Copy Markdown
Contributor

tolbertam commented Jul 11, 2016

By default, under "Preferences > Editor > Inspections > Java > Java language level migration aids", I had an inspection called "Usages of API which isn't available at the configured language level", set to Error severity.

I see, I don't have that option, but I do have Usages of API documented as @since 1.5 (1.6|1.7) which I bet is the same thing. It's currently unchecked but when I turn it on I see the errors you are referencing on the import statement.

@olim7t
JAVA-1161: Preserve full time zone info in ZonedDateTimeCodec and Dat…
b72d113 
…eTimeCodec

This commit contains contributions by Benoît Tellier (@chibenwa).
@olim7t
Copy link
Copy Markdown
Contributor

olim7t commented Jul 15, 2016

I've tried noinspection Since15 comments, but I can't find a way to apply them to the whole file. Another way would be to exclude the files using the Settings dialog, but that means storing IntelliJ settings in SCM which I'm not a fan of.
This is a minor problem, I vote to keep the fully-qualified names for now.

@olim7t olim7t merged commit c20f24b into 3.0 Jul 15, 2016
@olim7t olim7t deleted the java1161 branch July 15, 2016 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

reviewed_AT reviewed_OM

Projects

None yet

Milestone

3.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@adutra @tolbertam @olim7t