Skip to content

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