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 invalid ISO 8601 datetime fomat #51

Merged
merged 1 commit into from
Feb 6, 2021
Merged

Fix invalid ISO 8601 datetime fomat #51

merged 1 commit into from
Feb 6, 2021

Conversation

michael-o
Copy link
Contributor

SimpleDateFormat used Z as timezone, though this is invalid because it is a RFC
timezone and not an ISO 8601 timezone. Use XXX format for ISO-compliant timezone
format.

@basil3whitehouse
Copy link

Please add to the test (Iso8601DateTest) what is expected.

@michael-o
Copy link
Contributor Author

@basil3whitehouse Yes, will do.

SimpleDateFormat used Z as timezone, though this is invalid because it is a RFC
timezone and not an ISO 8601 timezone. Use XXX format for ISO-compliant timezone
format.
@michael-o
Copy link
Contributor Author

@basil3whitehouse I have added another test with Zulu timezone. I did not add another one because it won't get a test stable w/o some effort due to changing offsets and DSTs. Such a test would be brittle.

assertNotNull(date);

TimeZone savedTimeZone = TimeZone.getDefault();
TimeZone zuluTimeZone = TimeZone.getTimeZone("Etc/UTC");
Copy link
Contributor

@wwannemacher wwannemacher Sep 26, 2016

Choose a reason for hiding this comment

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

I was going to make a comment here about finding and using a constant, but decided to check online and make sure that constants existed... shame on you, Java, for not having constants for timezone IDs!

@michael-o
Copy link
Contributor Author

Anymore willing to merge after all?

@michael-o
Copy link
Contributor Author

anyone?

@DarthHater
Copy link
Member

@michael-o , hi there, I got sent this, and I'm so sorry it has taken so long for someone to reply! I don't really actively monitor this repo, so I completely missed it. @bhamail and I can likely get this over the hump.

Do me a favor, go and sign our CLA here, and let me know when you have. Once you have, I'll get this merged with someone! https://sonatypecla.herokuapp.com/sign-cla

@michael-o
Copy link
Contributor Author

@DarthHater Signed CLA, please check.

@DarthHater
Copy link
Member

@michael-o we are circulating this to the internal team to see what it impacts (since it's consumed further down the chain). Thank you! Sorry again for how long it's taken, I appreciate your contribution!

@mpiggott mpiggott merged commit 10f3852 into sonatype:master Feb 6, 2021
@mpiggott
Copy link
Contributor

mpiggott commented Feb 6, 2021

Thanks @michael-o I'm going to try to do some dependency updates and release in the next week.

@mpiggott
Copy link
Contributor

@michael-o I forgot to post this but I published 2.3.2-01 - https://search.maven.org/artifact/org.sonatype.goodies/goodies/2.3.2-01/pom

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.

5 participants