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-22463 Fix the conversion from gasoline-equivalent units to kilograms-per-meter-squared-per-second #2616

Merged
merged 1 commit into from
Sep 23, 2023
Merged

ICU-22463 Fix the conversion from gasoline-equivalent units to kilograms-per-meter-squared-per-second #2616

merged 1 commit into from
Sep 23, 2023

Conversation

younies
Copy link
Member

@younies younies commented Sep 21, 2023 •
edited
Loading

Correcting the conversion from gasoline-equivalent units to kilograms-per-meter-squared-per-second is essential. The issue doesn't lie with the Java segment, as Java code employs a more appropriate method for handling unit constants.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22463
  • 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

@younies younies requested a review from pedberg-icu September 21, 2023 16:04
younies added a commit to younies/icu that referenced this pull request Sep 21, 2023
@younies
ICU-22463 Fix gasoline-equivalent conversion
8f203e8 
See unicode-org#2616
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@younies younies requested a review from sffc September 21, 2023 16:22
@markusicu markusicu self-requested a review September 21, 2023 16:33
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4j/main/core/src/main/java/com/ibm/icu/impl/units/UnitsConverter.java is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

younies added a commit to younies/icu that referenced this pull request Sep 21, 2023
@younies
ICU-22463 Fix gasoline-equivalent conversion
fa13408 
See unicode-org#2616
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/units_converter.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@younies younies requested a review from sffc September 21, 2023 17:24
younies added a commit to younies/icu that referenced this pull request Sep 21, 2023
@younies
ICU-22463 Fix gasoline-equivalent conversion
df68f03 
See unicode-org#2616
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@younies younies changed the title ICU-22463 Fix gasoline-equivalent conversion ICU-22463 Fix the conversion from gasoline-equivalent units to kilograms-per-meter-squared-per-second Sep 21, 2023
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/units_converter.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

younies added a commit to younies/icu that referenced this pull request Sep 22, 2023
@younies
ICU-22463 Fix the conversion from gasoline-equivalent units to kilogr…
ee17917 
…ams-per-meter-squared-per-second

See unicode-org#2616
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

younies added a commit to younies/icu that referenced this pull request Sep 22, 2023
@younies
ICU-22463 Fix the conversion from gasoline-equivalent units to kilogr…
ee1de1e 
…ams-per-meter-squared-per-second

See unicode-org#2616
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

markusicu
markusicu approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

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

lgtm
@sffc ok?

sffc
sffc approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@sffc sffc left a comment •
edited
Loading

Choose a reason for hiding this comment

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

More tests would be nice (we can add some now without waiting for CLDR) but the logic looks correct to me now so let's merge it

@younies younies merged commit c468984 into unicode-org:main Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sffc sffc sffc approved these changes

@markusicu markusicu markusicu approved these changes

@pedberg-icu pedberg-icu Awaiting requested review from pedberg-icu

@richgillam richgillam Awaiting requested review from richgillam

Assignees

@sffc sffc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@younies @sffc @markusicu