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

Rewrite the testParsingDatesFormattedWithSystemLocale(), Fix #2199 #2287

Merged
merged 4 commits into from
Dec 16, 2022

Conversation

MaicolAntali
Copy link
Contributor

@MaicolAntali MaicolAntali commented Dec 16, 2022

I use the DataFormat to format the date. Now with different java versions (and different CLDR versions. More details) the test should not fail.

@eamonnmcmanus
Copy link
Member

This is great, thanks! One tiny formatting thing: the ); at the end of each assertion should be joined onto the end of the previous line, the way it was before:

      assertParsed(
          DateFormat.getDateTimeInstance(DateFormat.MEDIUM, DateFormat.MEDIUM).format(date),
          DateType.DATE.createDefaultsAdapterFactory());

It doesn't look as if the Google Java Style guide calls this out explicitly (or if it does I couldn't find it) but that is the general practice.

@MaicolAntali
Copy link
Contributor Author

oh yes, I'll take care of that real soon :)

@eamonnmcmanus
Copy link
Member

Thanks again!

@eamonnmcmanus eamonnmcmanus merged commit 6c12ded into google:master Dec 16, 2022
@MaicolAntali MaicolAntali deleted the fix-test-date-parsing branch December 20, 2022 09:10
tibor-universe pushed a commit to getuniverse/gson that referenced this pull request Jan 20, 2023
…#2199 (google#2287)

* Rewrite the `testParsingDatesFormattedWithSystemLocale()`, Fix google#2199

* Format the test

* Format the code following Google Java Style Guide

* Revert "Format the code following Google Java Style Guide"

This reverts commit f5e2e16.

(cherry picked from commit 6c12ded)
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