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 ISO week date numbering for english too #112801

Closed
wants to merge 1 commit into from

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Sep 12, 2024

TL;DR - dates are really, really complicated. Reference - https://en.wikipedia.org/wiki/ISO_week_date

With the changes to defaulting the locale to english rather than root (#112796, #112799), the Iso override added by #48209 no longer applies to english, which results in broken tests as the week numbering changes between the two locales. This PR fixes the immediate problem.

However, this opens up a rather large can of worms - what about all the other locales people use? I don't believe we have any defined behaviour around this, so what is the correct thing to do long-term, and do we want to break week numbering too? Do we just revert to whatever CLDR does, with the corresponding break here too?

This is not a straightforward change - there may be users explicitly using en locale with non-iso week dates that will be broken by this PR.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@thecoop thecoop requested a review from a team September 12, 2024 10:40
@prdoyle
Copy link
Contributor

prdoyle commented Sep 12, 2024

English and Root use different week numbering schemes? Is either of them compliant with ISO week numbering?

@thecoop
Copy link
Member Author

thecoop commented Sep 12, 2024

They are both first-week-sunday, 1 day minimum week, on compat and cldr. But we override it for compat root, which is where this discrepancy comes in.

There are also other changes in other locales between compat and cldr.

@thecoop
Copy link
Member Author

thecoop commented Sep 20, 2024

We're going to remove the Iso implementation instead

@thecoop thecoop closed this Sep 20, 2024
@thecoop thecoop deleted the iso-week-starts branch September 20, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants