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.

Sign up for GitHub

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

Jump to bottom

ICU-22908 MF2: Update spec tests and update implementation for recent spec changes #3198

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ICU-22908 MF2: Update spec tests and update implementation for recent spec changes #3198

wants to merge 1 commit into from

Conversation

catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Sep 21, 2024 •
edited
Loading

Updating the spec tests requires two implementation changes:

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22908
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@catamorphism
Copy link
Contributor Author

Note: this PR updates the shared tests, but doesn't update the ICU4J implementation with the two spec changes, so ICU4J tests will fail in CI for now.

I'll check with @mihnita about the Java side.

@mihnita
Copy link
Contributor

mihnita commented Sep 21, 2024

We also have this new fresh issue filed:
https://unicode-org.atlassian.net/browse/ICU-22906

I am almost tempted to separate the tests again...

@mihnita
Copy link
Contributor

mihnita commented Sep 21, 2024

I am tempted to say "let's drop it"

We were supposed to be code freeze, and this is something that my superstition feels very strongly: submit on Friday evening :-)

Even if it lands on Monday, it is still "Friday afternoon" with respect to the ICU release schedule.

@mihnita mihnita mentioned this pull request Sep 22, 2024
Draft
7 tasks
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/test/intltest/messageformat2test_read_json.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@catamorphism catamorphism mentioned this pull request Oct 8, 2024
Open
7 tasks
@catamorphism
ICU-22908 MF2: Update spec tests and update implementation for recent…
ed61179 
… spec changes

Updating the spec tests requires two implementation changes:
* Match on variables rather than expressions --
  see unicode-org/message-format-wg#877
* Require attribute values to be literals (if present) --
  see unicode-org/message-format-wg#894

This updates the spec tests to commit 6c3704f41a9c24427126429fb43992b03609dfc8
in https://github.com/unicode-org/message-format-wg/ . Any changes
following that commit will be addressed in a future PR.
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/messageformat2_checker.cpp is different
  • icu4c/source/i18n/messageformat2_checker.h is different
  • icu4c/source/i18n/messageformat2_data_model.cpp is different
  • icu4c/source/i18n/messageformat2_parser.cpp is different
  • icu4c/source/i18n/messageformat2_parser.h is different
  • icu4c/source/i18n/messageformat2_serializer.cpp is different
  • icu4c/source/i18n/messageformat2.cpp is different
  • icu4c/source/i18n/unicode/messageformat2_data_model.h is different
  • icu4c/source/i18n/unicode/messageformat2.h is different
  • icu4c/source/test/intltest/messageformat2test_custom.cpp is different
  • icu4c/source/test/intltest/messageformat2test_read_json.cpp is different
  • icu4c/source/test/intltest/messageformat2test_utils.h is different
  • icu4c/source/test/intltest/messageformat2test.cpp is different
  • testdata/message2/normalization.json is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@catamorphism catamorphism marked this pull request as ready for review November 14, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@echeran echeran Awaiting requested review from echeran

@mihnita mihnita Awaiting requested review from mihnita

@srl295 srl295 Awaiting requested review from srl295

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@catamorphism @mihnita