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

Fix date format for non-english locale #52

Merged
merged 2 commits into from
Jun 22, 2018
Merged

Conversation

fpaparoni
Copy link
Contributor

No description provided.

@moderakh
Copy link
Contributor

@fpaparoni thanks for the PR.

I wonder if we can have a unit test for this which Utils.nowAsRFC1123() returns the correct value for Non US Locale? Can you please add that too?

@fpaparoni
Copy link
Contributor Author

Utils.nowAsRFC1123() now always returns a value for Locale.US because otherwise it isn't accepted by cosmosdb, so I don't understand a test for non US Locale.

@moderakh
Copy link
Contributor

I assume on non-english locale in the absence of your PR Utils.nowAsRFC1123() returns invalid type format. am I right?

if so then maybe we can have a unit test which sets the non-english locale in the JVM, gets the value of validate Utils.nowAsRFC1123() time and format, and then set back default locale in the JVM.

This test will pass for your PR, but will fail in the absence of your PR.
Does this make sense?

@msftclas
Copy link

msftclas commented Jun 21, 2018

CLA assistant check
All CLA requirements met.

@fpaparoni
Copy link
Contributor Author

fpaparoni commented Jun 21, 2018

Ok I understood the purpose of it and I pushed the unit test

DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss zzz",Locale.US);
String time=Utils.nowAsRFC1123();
Locale.setDefault(Locale.US);
RFC_1123_DATE_TIME.parse(time);
Copy link
Contributor

Choose a reason for hiding this comment

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

does RFC_1123_DATE_TIME.parse(time) fail without your bugfix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes because Utils.nowAsRFC1123 without bugfix returns time depending on Locale of your enviroment. Setting it to a Locale different from US and expecting to parse this String will throw an Exception of date format

@moderakh moderakh merged commit 594622c into Azure:master Jun 22, 2018
@moderakh
Copy link
Contributor

Thank you @fpaparoni merging this PR.
This will be in the next release.

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