Skip to content

remove joda-time#18035

Open
dutta-sh wants to merge 7 commits into
prestodb:masterfrom
dutta-sh:refactor/redis_joda
Open

remove joda-time#18035
dutta-sh wants to merge 7 commits into
prestodb:masterfrom
dutta-sh:refactor/redis_joda

Conversation

@dutta-sh

Copy link
Copy Markdown

Using Java 8 java.time packages to replace joda-time

@ajaygeorge

Copy link
Copy Markdown
Contributor

@dutta-sh Can you please squash the commits

public long getLong()
{
long millis = FORMATTER.parseMillis(getSlice().toStringAscii());
TemporalAccessor temporal = FORMATTER.parseBest(getSlice().toStringAscii(), LocalDateTime::from, LocalDate::from);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand why we need parseBest here.

@dutta-sh dutta-sh Jul 14, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String representing a date (with or without time) needs two separate formatters, or needs to use parseBest. More context here - https://stackoverflow.com/questions/48280447/java-8-datetimeformatter-with-optional-part

Comment thread presto-redis/src/test/java/com/facebook/presto/redis/util/RedisLoader.java Outdated
Comment thread presto-redis/src/test/java/com/facebook/presto/redis/util/RedisLoader.java Outdated
Comment thread presto-redis/src/test/java/com/facebook/presto/redis/util/RedisLoader.java Outdated
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