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

style: rename crates to kebab-case #12118

Merged
merged 6 commits into from
May 1, 2022
Merged

Conversation

randomicon00
Copy link
Contributor

Ref: #12102
I updated all the folders names as well as the crates names in each Cargo.toml to use kebab-case.

This is my first ra PR. In case I missed something, I am ready/available to fix it until it is ready to merge.

Thank you.

@lnicola
Copy link
Member

lnicola commented Apr 29, 2022

Can you rebase? It seems there's a conflict on one file.

@randomicon00
Copy link
Contributor Author

Yes, I can. My current branch doesn't include the latest 4 changes.

@lnicola
Copy link
Member

lnicola commented Apr 29, 2022

Great. We can't merge this because one of the newer changes touches a file in ide_db, which you've renamed.

@lnicola
Copy link
Member

lnicola commented Apr 29, 2022

changelog internal (first contribution) rename crates to kebab-case

@jonas-schievink
Copy link
Contributor

Looks like you rebased master onto your branch, not your branch onto master

@randomicon00
Copy link
Contributor Author

Yes, as I rarely use rebase, I've made the mistake and instantly regretted it. I secretly hoped it will go away (this trick never works btw). It came back to haunt me through your message @jonas-schievink :)

@lnicola
Copy link
Member

lnicola commented Apr 30, 2022

@randomicon00 I think there's still a failing test that checks that a generated file is up to date. Do you need help with this?

@randomicon00
Copy link
Contributor Author

randomicon00 commented Apr 30, 2022

@lnicola I will have a look at it tonight or tomorrow morning and let you know. The test that fails is tests::sourcegen::source_genassists_docs in ide-assists crate. Currently updating the crates names in tests that fail.

@lnicola
Copy link
Member

lnicola commented May 1, 2022

Looks good. If it's not too much of a hassle, can you do a final squash to get rid of the swapfile?

@bors delegate+

@bors
Copy link
Contributor

bors commented May 1, 2022

✌️ @randomicon00 can now approve this pull request

@bors
Copy link
Contributor

bors commented May 1, 2022

☔ The latest upstream changes (presumably #12099) made this pull request unmergeable. Please resolve the merge conflicts.

@randomicon00
Copy link
Contributor Author

As much as the task is trivial, as much as branch conflits seem to keep rolling!
@lnicola what would be, please, the best approach to resolve these conflicts?

@lnicola
Copy link
Member

lnicola commented May 1, 2022

I would squash, then rebase and fix the conflicts. Then merge it without waiting for a review (leave a comment with @ bors r=lnicola).

@randomicon00
Copy link
Contributor Author

all tests are passing. @bors r=lnicola

@bors
Copy link
Contributor

bors commented May 1, 2022

📌 Commit aade319 has been approved by lnicola

@bors
Copy link
Contributor

bors commented May 1, 2022

⌛ Testing commit aade319 with merge 5c88d93...

@bors
Copy link
Contributor

bors commented May 1, 2022

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing 5c88d93 to master...

@Veykril
Copy link
Member

Veykril commented May 1, 2022

Good thing we only had ~20 open PRs and not more 😅

@lnicola
Copy link
Member

lnicola commented May 2, 2022

Nope, looks like you didn't actually squash the changes 😄

image

@randomicon00
Copy link
Contributor Author

randomicon00 commented May 2, 2022

Good morning! I squashed the last 6 commits.

@pksunkara
Copy link
Contributor

This broke the publish action as you can see by https://github.com/rust-lang/rust-analyzer/runs/6524727008?check_suite_focus=true#step:5:223. cc @jonas-schievink

I am not sure why this change was made.

@Veykril
Copy link
Member

Veykril commented May 22, 2022

This change was made for consistency, we had some crates using kebab-case and some using snake_case before.

@pksunkara
Copy link
Contributor

So, how do we fix the old crates being named with _? Crates.io is not allowing us to publish with -, I think.

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.

6 participants