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

Add imports_granularity = "Module" to rustfmt.toml #11095

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

Alexendoo
Copy link
Member

This lets rustfmt split/merge imports, Module seems to be the most common style in clippy

https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#imports_granularity

changelog: none

Almost all the updates other than the config file change are from cargo dev fmt or blessed tests, the exceptions being

  • tests/ui/single_component_path_imports.rs
  • tests/ui/single_component_path_imports_nested_first.rs
  • tests/ui/single_component_path_imports_self_after.rs
  • tests/ui/single_component_path_imports_self_before.rs
  • tests/ui/unsafe_removed_from_name.rs (added a test with merged imports as a drive by)
  • tests/ui/wildcard_imports.rs
  • tests/ui/wildcard_imports_2021.rs

@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2023

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 3, 2023
@Centri3
Copy link
Member

Centri3 commented Jul 3, 2023

What about group_imports? I see something akin to StdExternalCrate used often but that's likely because of what ra does automatically

@flip1995
Copy link
Member

flip1995 commented Jul 3, 2023

I'd like to switch to rust-lang/rustfmt#5781 if that should get merged.

@Centri3
Copy link
Member

Centri3 commented Jul 3, 2023

Ooh I like that :)

@Alexendoo
Copy link
Member Author

Alexendoo commented Jul 3, 2023

FWIW this also only modifies just under a third of the files, 195/605 (excluding tests since almost all of those are unchanged)

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

I'm fine with adding this. But once the above linked feature is available, I'd like to move to that one. But I wouldn't block this PR on that.

Leaving this PR open for 1-3 days, so that people can raise their objections, if any.

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

same opinion as @flip1995 , r=me after a couple days

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

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

@Alexendoo
Copy link
Member Author

Let's create a bunch of conflicts!

@bors r=Manishearth

@bors
Copy link
Collaborator

bors commented Jul 13, 2023

📌 Commit 2811eff has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 13, 2023

⌛ Testing commit 2811eff with merge 7ccf5d4...

@bors
Copy link
Collaborator

bors commented Jul 13, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 7ccf5d4 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants