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

CLDR-15113 json: move datetimeSkeleton to a parallel location #1558

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 25, 2021

CLDR-15113

  • This PR completes the ticket. (yet again!)

Change from the previous stopgap format in #1552 to:

"dateFormats": {
  "full": "EEEE, MMMM d, y",
  "long": "MMMM d, y",
  "medium": "MMM d, y",
  "short": "M/d/yy"
},
"datetimeSkeleton": {
  "full": "ahmmsszzzz",
  "long": "ahmmssz",
  "medium": "ahmmss",
  "short": "ahmm"
},

@macchiati this is slightly different than your proposal in #1552 (comment) - I made the object name datetimeSkeleton to match the actual element name in CLDR, and the structure would allow a timeSkeleton etc if such was needed.

@srl295 srl295 requested review from sffc and macchiati October 25, 2021 21:21
@srl295 srl295 self-assigned this Oct 25, 2021
Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

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

I was proposing

dateFormats: {...}
dateSkeletons: {...}

Which also leads to the corresponding:

timeFormats: {...}
timeSkeletons: {...}

It is a bit hard to see what your code is producing, so can you check to see that it matches the above?

@srl295
Copy link
Member Author

srl295 commented Oct 25, 2021

@macchiati What I produce is in the head comment above, since datetimeSkeleton better matches the element name in XML. But I can change it to dateSkeletons exactly, will do.

- dateSkeletons and timeSkeletons are now siblings to dateFormats and timeFormats
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/cldr-code/src/main/resources/org/unicode/cldr/json/pathTransforms.txt is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@srl295
Copy link
Member Author

srl295 commented Oct 25, 2021

actually i missed the timeSkeletons before.

@macchiati as of 72536cc the output is now:

            "dateFormats": {
              "full": "EEEE, MMMM d, y",
              "long": "MMMM d, y",
              "medium": "MMM d, y",
              "short": "M/d/yy"
            },
            "dateSkeletons": {
              "full": "yMMMMEEEEd",
              "long": "yMMMMd",
              "medium": "yMMMd",
              "short": "yyMd"
            },
            "timeFormats": {
              "full": "h:mm:ss a zzzz",
              "long": "h:mm:ss a z",
              "medium": "h:mm:ss a",
              "short": "h:mm a"
            },
            "timeSkeletons": {
              "full": "ahmmsszzzz",
              "long": "ahmmssz",
              "medium": "ahmmss",
              "short": "ahmm"
            },

@srl295 srl295 requested a review from macchiati October 25, 2021 22:08
@srl295
Copy link
Member Author

srl295 commented Oct 25, 2021

@macchiati this is the full diff for output files: (i skipped changing the version this time) unicode-org/cldr-json@a18f696

Copy link
Member

@macchiati macchiati 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 now

@srl295 srl295 merged commit f022bde into unicode-org:maint/maint-40 Oct 26, 2021
@srl295 srl295 deleted the cldr-15113/datetimeskel2 branch October 26, 2021 01:25
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.

2 participants