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

Use _ in crate names #304

Closed
sffc opened this issue Oct 6, 2020 · 12 comments
Closed

Use _ in crate names #304

sffc opened this issue Oct 6, 2020 · 12 comments
Assignees
Labels
C-meta Component: Relating to ICU4X as a whole T-core Type: Required functionality
Milestone

Comments

@sffc
Copy link
Member

sffc commented Oct 6, 2020

We are using hyphens in package names, but underscores pretty much everywhere else:

  • File names
  • Crate names
  • Module names
  • Function names

I don't understand why we don't use underscores in package names, too. There are lots of Rust packages that use underscores, like serde_json. This would eliminate the oddness that you put "icu-locale" in Cargo.toml, but then when you use it in code, you need to write "icu_locale".

@sffc sffc added the discuss Discuss at a future ICU4X-SC meeting label Oct 6, 2020
@Manishearth
Copy link
Member

I'm not sure if there's ecosystem-wide agreement on what to do, but my impression is that people prefer to use dashes. There are open bugs to make crates.io, cargo, and docs.rs more agnostic to dashes-vs-underscores (you already cannot publish crates that only differ on dashes vs underscores). Dashes look nicer so folks tend to use them more.

@sffc sffc removed the discuss Discuss at a future ICU4X-SC meeting label Oct 9, 2020
@zbraniecki
Copy link
Member

Consensus is to:

  • switch to _ for crate names to reduce confusion between names and imports.
  • rename pluralrules to plurals and unicodeset to uniset.

@zbraniecki zbraniecki changed the title Can we use underscores everywhere please? Consistent naming of crates Oct 9, 2020
@zbraniecki zbraniecki added this to the ICU4X 0.1 milestone Oct 9, 2020
@zbraniecki zbraniecki mentioned this issue Oct 9, 2020
@Manishearth
Copy link
Member

Manishearth commented Oct 9, 2020

switch to _ for crate names to reduce confusion between names and imports.

Is there really confusion? - doesn't work in imports so it's obvious it needs to be _, and when I type a crate I almost always try - first. If anything it's confusing when the crate name contains _ because most crates tend to use -.

@zbraniecki
Copy link
Member

Is there really confusion?

If the choice is between always use _ and use _ in X, and use - in Y then the former is more consistent and simpler, right?

If Rust settled the dispute and decided to go for - in crate names, I believe we would follow. With Rust Core Group not making any decisions and crates doing both, it becomes an arbitrary choice and we'd prefer to go for what is simpler for us if it's wild west of namings anyway.

How strongly you feel about - in crate names? Would you like us to postpone renames and discuss? Would you be able to provide any justification beyond it looks better that counters the its consistent?

@Manishearth
Copy link
Member

If Rust settled the dispute and decided to go for - in crate names, I believe we would follow.

The community has largely decided on -, with _ being used more in older crates, but it's not 100% decided.

Here are the counts of - vs _:

[16:49:22] मanishearth@noether ~/sand/crates.io-index ^_^ 
$ find . -type f | sed "s/.*\///g" | tr -d -c "_" | wc -c
10901
[16:49:45] मanishearth@noether ~/sand/crates.io-index ^_^ 
$ find . -type f | sed "s/.*\///g" | tr -d -c "-" | wc -c
24828

Would you be able to provide any justification beyond it looks better that counters the its consistent?

Yes, because - is more common than _ except for older crates and some holdouts, people typically try for - first, and when that doesn't work they try the other one.

Some namespacing proposals also involve specially blessing -, though they are unlikely to pass.

@zbraniecki zbraniecki changed the title Consistent naming of crates Use _ in crate names Oct 10, 2020
@zbraniecki
Copy link
Member

I'm going to split off the short names from _ issue ito #322 and keep this issue for _ vs -

@sffc
Copy link
Member Author

sffc commented Oct 12, 2020

It sounds to me like the main advantage of - is:

people typically try for - first, and when that doesn't work they try the other one.

Here's my counter-argument. When I add new dependencies to my Cargo.toml file, I usually copy and paste the crate name from some web site, like crates.io, docs.rs, etc. It's easier to copy a name with underscores, because on most browsers and operating systems, - break words but _ do not, so you can double-click a string with _ and you get the whole string highlighted. Try it out:

foo-bar or foo_bar

I'm not saying that arguments about developers' expectations are invalid, but if we're looking at developer experience as the primary decision point, there are advantages of _, too.

@mihnita
Copy link
Contributor

mihnita commented Oct 12, 2020

Another argument for _

If using hyphen somehow propagates to file / folder names, that would be a problem for some build systems (for example bazel (home), Bazel (Wikipedia))

Bazel takes the files / folders and use the names "as is" to create python variable names.
So all files / folders have to be valid python identifiers.
No hyphen, apostrophes, spaces, etc.

Kind of a shame for a system used in this day and age, but this is the current status.

Everything we bring inside Google has to build with bazel (the internal version).
And I've had great pain doing dealing with hyphen (and spaces, and apostrophes) in file / folder names for other projects.

@zbraniecki
Copy link
Member

@Manishearth @mihnita @sffc and I met today to discuss this topic. The conclusion is as follows:

  • Short term (0.1)
    • Rename locale to locid
    • Replace - with _ in crate and directory names
  • Mid-term
    • Manish is going to write a pre-RFC tomorrow for further blurring the line between - and _ in crate names
  • Long-term

Thank you all, and I'll write the PRs for the short-term renames as one of the last steps before 0.1 release to minimize the PRs bitrotting.

@Manishearth
Copy link
Member

* Cargo and Rust are going to introduce namespaces based on Manish proposal - https://internals.rust-lang.org/t/pre-rfc-packages-as-optional-namespaces/13059

This isn't for certain, fwiw, this is just my personal endeavor

@zbraniecki
Copy link
Member

This is now complete.

@sffc sffc added C-meta Component: Relating to ICU4X as a whole T-core Type: Required functionality labels Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-meta Component: Relating to ICU4X as a whole T-core Type: Required functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants