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

Move icu_num_utils to utils/num #303

Merged
merged 4 commits into from
Oct 6, 2020
Merged

Conversation

zbraniecki
Copy link
Member

Fixes #167.

This just moves the icu_num_utils component to be utils/num and renames it to icu_utils_num.

I'm open to discuss whether the latter is necessary. My motivation is that it allows us to collect all utils as icu_utils and num_utils is just one of them.

@zbraniecki zbraniecki requested review from sffc and a team as code owners October 6, 2020 01:16
@zbraniecki
Copy link
Member Author

An open question is whether we want to have utils/fixed_decimal and icu-utils-fixed-decimal (or just fixed-decimal)? I'm not sure how many utils for numbers will we have and is utils/num and icu-utils-num just an alias for fixed-decimal or for a collection of APIs that fixed-decimal is just one of.

@coveralls
Copy link

coveralls commented Oct 6, 2020

Pull Request Test Coverage Report for Build a3d04779625fdf536de0dcd2d6cd16363142e69c-PR-303

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 71 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-6.03%) to 77.652%

Files with Coverage Reduction New Missed Lines %
components/data-provider/src/data_entry.rs 1 86.27%
components/locale/src/parser/errors.rs 1 62.5%
components/locale/src/subtags/variants.rs 1 85.71%
components/data-provider/src/structs/decimal.rs 2 23.53%
components/data-provider/src/structs/plurals.rs 2 75.0%
components/locale/src/extensions/transform/fields.rs 2 89.47%
components/locale/src/extensions/unicode/attributes.rs 2 85.71%
components/locale/src/extensions/unicode/key.rs 2 91.3%
components/locale/src/extensions/unicode/keywords.rs 2 84.62%
components/locale/src/extensions/unicode/value.rs 2 92.68%
Totals Coverage Status
Change from base Build b100859a1e2ff1b89193c2cdbb484c199c56b591: -6.03%
Covered Lines: 2891
Relevant Lines: 3723

💛 - Coveralls

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • components/pluralrules/src/operands.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@sffc
Copy link
Member

sffc commented Oct 6, 2020

+1 for utils/fixed_decimal.

The package name should be either "icu-fixed-decimal" or "fixed-decimal", whichever makes you happier.

@zbraniecki
Copy link
Member Author

great! I named it fixed-decimal in line with other crates, and if we decide to go with underscores in #304, we'll rename it together with all other crates.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/perf-runtime.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

sffc
sffc previously approved these changes Oct 6, 2020
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

Thanks!

utils/fixed-decimal/src/lib.rs Outdated Show resolved Hide resolved
@zbraniecki zbraniecki merged commit 06aecd5 into unicode-org:master Oct 6, 2020
@zbraniecki zbraniecki deleted the fd-util branch October 19, 2020 15:49
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.

FixedDecimal as a standalone crate
3 participants