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

Use explicit Locale for era formatting #974

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Conversation

busti
Copy link
Contributor

@busti busti commented Sep 11, 2023

The field eraFormatter in TemporalCodecs.scala is missing an explicit reference to Locale.US which can cause unexpected behavior when a locale db with i.e. british locale is added to the dependencies.

I encountered this because we added https://github.com/cquiroz/scala-java-locales to our project, whose default db, which is recommended in the repository, is for british english.

@armanbilge
Copy link
Member

Thanks, should we target this to series/0.6.x branch?

@armanbilge
Copy link
Member

armanbilge commented Sep 11, 2023

whose default db, which is recommended in the repository, is for british english.

Interesting. Where exactly is this, can you point me to it?

Edit: ah, this bit in the README?

scala-java-locales will try to guess the locale but if it can't or it is not not the locales db it sets en (English) as the default locale. This is a design decision to support the many API calls that require a default locale. It seems that Scala.js de facto uses en for number formatting.

@busti
Copy link
Contributor Author

busti commented Sep 12, 2023

Specifically this arose because we were using $date from TemporalCodecs in a query which before adding the library formatted a date as 2023-03-12 and as 2023-03-12 CE after which postgres wasn't able to process.

@mpilquist mpilquist merged commit a394ad3 into typelevel:main Oct 27, 2023
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