Skip to content

Add calendar and numberingSystem options to date/time #949

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

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

macchiati
Copy link
Member

@macchiati macchiati commented Nov 19, 2024

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.

@aphillips
Copy link
Member

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 MF2.0 Draft Candidate and removed Action-Item Action item assigned by the WG labels Nov 19, 2024
@macchiati
Copy link
Member Author

LGTM. Shall I go ahead and merge?

Copy link
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.

Copy link
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:

> [!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)

@aphillips
Copy link
Member

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

Copy link
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.

Co-authored-by: Eemeli Aro <[email protected]>
@aphillips aphillips merged commit 6aad447 into main Nov 19, 2024
1 check passed
@aphillips aphillips deleted the macchiati-patch-1 branch November 19, 2024 17:01
Copy link
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
functions Issue pertains to the default function set LDML46.1 MF2.0 Draft Candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants