Skip to content

Add calendar and numberingSystem options to date/time#949

Merged
aphillips merged 3 commits intomainfrom
macchiati-patch-1
Nov 19, 2024
Merged

Add calendar and numberingSystem options to date/time#949
aphillips merged 3 commits intomainfrom
macchiati-patch-1

Conversation

@macchiati
Copy link
Copy Markdown
Member

@macchiati macchiati commented Nov 19, 2024 •
edited
Loading

This is as per the discussion in the WG, separating them out from #911

Two changes from that:

  1. Used RECOMMENDED instead of Optional, as per Change standard/optional to required/recommended #946
  2. I noticed also there that RECOMMENDED needs a maturity status, so added a placeholder for that, with Proposed. Since that is only in exploration/maintaining-registry.md, I presume it would be removed for release.

@macchiati macchiati mentioned this pull request Nov 19, 2024
Merged
@aphillips
Copy link
Copy Markdown
Member

aphillips commented Nov 19, 2024

I resolved the conflict. In doing so, I reordered the proposed options below the REQUIRED one and rephrased the text slightly to take advantage of the fact that REQUIRED and RECOMMENDED are 2119 keywords. Please have a look.

@aphillips aphillips added functions Issue pertains to the default function set Action-Item Action item assigned by the WG LDML46.1 and removed Action-Item Action item assigned by the WG labels Nov 19, 2024
@macchiati
Copy link
Copy Markdown
Member Author

macchiati commented Nov 19, 2024

LGTM. Shall I go ahead and merge?

eemeli
eemeli approved these changes Nov 19, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

Looks good, pending the inline suggestion to drop the maturity level mention.

@eemeli eemeli mentioned this pull request Nov 19, 2024
Closed
eemeli
eemeli requested changes Nov 19, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

At least the calendar and numberingSystem mentions should also be removed from this note:

message-format-wg/spec/registry.md

Lines 952 to 964 in 960f800

> [!NOTE]
> The following options do not have default values because they are only to be used
> as overrides for locale-and-value dependent implementation-defined defaults.
The following date/time options are **not** part of the default registry.
Implementations SHOULD avoid creating options that conflict with these, but
are encouraged to track development of these options during Tech Preview:
- `calendar` (default is locale-specific)
- valid [Unicode Calendar Identifier](https://cldr-smoke.unicode.org/spec/main/ldml/tr35.html#UnicodeCalendarIdentifier)
- `numberingSystem` (default is locale-specific)
- valid [Unicode Number System Identifier](https://cldr-smoke.unicode.org/spec/main/ldml/tr35.html#UnicodeNumberSystemIdentifier)
- `timeZone` (default is system default time zone or UTC)
- valid identifier per [BCP175](https://www.rfc-editor.org/rfc/rfc6557)

@eemeli eemeli mentioned this pull request Nov 19, 2024
Merged
@aphillips
Copy link
Copy Markdown
Member

aphillips commented Nov 19, 2024

@eemeli I will make a separate PR to remove the note.

eemeli
eemeli approved these changes Nov 19, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

I'm fine with removing the note separately.

@aphillips @eemeli
Update spec/registry.md
a1d1f77 
Co-authored-by: Eemeli Aro <[email protected]>
@aphillips aphillips merged commit 6aad447 into main Nov 19, 2024
@aphillips aphillips deleted the macchiati-patch-1 branch November 19, 2024 17:01
mihnita
mihnita reviewed Nov 21, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mihnita mihnita left a comment

Choose a reason for hiding this comment

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

For some reason I can review, but can't approve.
My only button is "Submit review"

So: approved

@eemeli eemeli added this to the LDML 46.1 milestone Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mihnita mihnita mihnita left review comments

@aphillips aphillips aphillips approved these changes

@eemeli eemeli eemeli approved these changes

@catamorphism catamorphism Awaiting requested review from catamorphism

Assignees

No one assigned

Labels

functions Issue pertains to the default function set

Projects

None yet

Milestone

LDML 46.1

Development

Successfully merging this pull request may close these issues.

4 participants

@macchiati @aphillips @eemeli @mihnita