Skip to content

Conversation

sakebook
Copy link
Contributor

@sakebook sakebook commented Nov 6, 2019

Discussion

No issue.

Current Date test is depend on Locale(GMT).

I ran the test on my local machine and it was not successful.
This is because the Japanese locale (JMT) is used.

I defined use Locale.US.

Testing

Fixed test

API Changes

None

RELEASE NOTE: Fixed a bug in date string parsing during error handling.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks @sakebook. Just one change needed, and then we can merge.

new SimpleDateFormat(PATTERN_RFC1123),
new SimpleDateFormat(PATTERN_RFC1036),
new SimpleDateFormat(PATTERN_ASCTIME)
new SimpleDateFormat(PATTERN_RFC1123, Locale.UK),
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it. 3e8ec14

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hiranya911 hiranya911 assigned hiranya911 and unassigned sakebook Nov 6, 2019
@hiranya911 hiranya911 merged commit 76f0f95 into firebase:master Nov 6, 2019
@hiranya911 hiranya911 changed the title Fix Date test in another Locale fix: Date test in another Locale Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants