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

Short names #318

Closed
wants to merge 1 commit into from
Closed

Short names #318

wants to merge 1 commit into from

Conversation

zbraniecki
Copy link
Member

Fixes #304.
Depends on #311

@zbraniecki zbraniecki requested review from nciric, sffc and a team as code owners October 9, 2020 19:22
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • CODEOWNERS is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@zbraniecki zbraniecki added this to the ICU4X 0.1 milestone Oct 9, 2020
sffc
sffc reviewed Oct 9, 2020
View reviewed changes
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.

The package names for the data provider crates are long. We should consider taking this opportunity to shorten them as well. My suggestion:

  • icu_testdata (status quo)
  • icu_data (replaces icu_data_provider)
  • icu_data_cldr (replaces icu_cldr_json_data_provider)
  • icu_data_fs (replaces (icu_fs_data_provider)

components/cldr_json_data_provider/Cargo.toml
@@ -1,5 +1,5 @@
[package]
name = "icu-cldr-json-data-provider"
name = "icu_cldr_json_data_provider"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. "icu_data_cldr"?

components/data_provider/Cargo.toml
@@ -1,5 +1,5 @@
[package]
name = "icu-data-provider"
name = "icu_data_provider"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just "icu_data", and then the class is called DataProvider?

@zbraniecki
Copy link
Member Author

The package names for the data provider crates are long. We should consider taking this opportunity to shorten them as well. My suggestion:

* icu_testdata (status quo)

* icu_data (replaces icu_data_provider)

* icu_data_cldr (replaces icu_cldr_json_data_provider)

* icu_data_fs (replaces (icu_fs_data_provider)

I'd rather go for icu_provider, icu_cldr_provider, icu_fs_provider.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • components/datetime/benches/datetime.rs is no longer changed in the branch
  • components/datetime/benches/fixtures/tests/styles.json is no longer changed in the branch
  • components/datetime/examples/work_log.rs is no longer changed in the branch
  • components/datetime/tests/datetime.rs is no longer changed in the branch
  • components/datetime/tests/fixtures/data/icu4x/dates/gregory@1/en.json is no longer changed in the branch
  • components/datetime/tests/fixtures/data/icu4x/dates/gregory@1/pl.json is no longer changed in the branch
  • components/datetime/tests/fixtures/data/icu4x/dates/manifest.json is no longer changed in the branch
  • components/datetime/tests/fixtures/data/icu4x/manifest.json is no longer changed in the branch
  • components/datetime/tests/fixtures/tests/styles.json is no longer changed in the branch
  • components/fs_data_provider/tests/test_file_io.rs is different
  • components/fs-data-provider/tests/testdata/json_plurals_37/manifest.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/ar.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/de.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/en.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/iw.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/sr.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/und.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/cardinal@1/zh.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/ar.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/en.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/es.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/he.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/sr.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/und.json is no longer changed in the branch
  • components/fs-data-provider/tests/testdata/json_plurals_37/plurals/ordinal@1/yue.json is no longer changed in the branch
  • components/pluralrules/benches/fixtures/plurals.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/manifest.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/ar.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/cs.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/de.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/en.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/es.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/fr.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/it.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/pl.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/sk.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/uk.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/cardinal@1/zh.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/ar.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/cs.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/de.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/en.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/es.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/fr.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/it.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/pl.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/sk.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/uk.json is no longer changed in the branch
  • components/pluralrules/tests/data/json_plurals_37/plurals/ordinal@1/zh.json is no longer changed in the branch
  • components/pluralrules/tests/plurals.rs is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@filmil
Copy link
Contributor

filmil commented Oct 9, 2020 via email

@sffc
Copy link
Member

sffc commented Oct 9, 2020

icu_provider works for me.

If we want to reduce disruptions to merging, we could start by renaming the packages and modules but keeping the file system paths the same.

@sffc sffc marked this pull request as draft October 10, 2020 02:55
@sffc
Copy link
Member

sffc commented Oct 10, 2020

@zbraniecki says he'll revisit this PR next week, so @filmil you are unblocked.

@zbraniecki
Copy link
Member Author

I'm going to recreate a new PR based on the outcome of #304.

@zbraniecki zbraniecki closed this Oct 12, 2020
@zbraniecki zbraniecki deleted the short-names branch October 19, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sffc sffc sffc left review comments

@nciric nciric Awaiting requested review from nciric

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
ICU4X 0.1
Development

Successfully merging this pull request may close these issues.

Use _ in crate names
3 participants
@zbraniecki @filmil @sffc