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

Pull in nightly rust-fmt, use vid rustfmt.toml #2700

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pls148
Copy link
Contributor

@pls148 pls148 commented Feb 28, 2025

Old HS repo used to have nightly rustfmt. After some discussion in the monorepo channel, I figured we'd try nightly rustfmt to get better grouping of dependencies. This gets rid of the mess of redundant "use hotshot_types::a::b::some thing".

Tested locally with nix/direnv. Not sure if I got other peoples' use cases.

The main changes are to the following files:

  • flake.nix
  • rust-toolchain.toml
  • rustfmt.toml

The rest of the changes were done by rustfmt picking up the new config.

@pls148 pls148 force-pushed the ps/rustfmt-nightly branch 2 times, most recently from 4c5479e to a96ed35 Compare February 28, 2025 17:14
Copy link
Collaborator

@sveitser sveitser left a comment

Choose a reason for hiding this comment

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

Looks great thanks.

For next time, making a commit first with the changes and a separate commit for the formatting makes it easier to review because the reviewer can check out the first commit and run just fmt then diff against the branch HEAD. I did this manually now which (still) seemed a lot easier than reading at the diff.

One question, hotshot had group_imports = "StdExternalCrate" which we don't have here. I think that's quite nice. Any particular reason for excluding it?

@pls148 pls148 force-pushed the ps/rustfmt-nightly branch 4 times, most recently from 6149eba to 64c088c Compare March 4, 2025 17:18
@pls148
Copy link
Contributor Author

pls148 commented Mar 4, 2025

Looks great thanks.

For next time, making a commit first with the changes and a separate commit for the formatting makes it easier to review because the reviewer can check out the first commit and run just fmt then diff against the branch HEAD. I did this manually now which (still) seemed a lot easier than reading at the diff.

One question, hotshot had group_imports = "StdExternalCrate" which we don't have here. I think that's quite nice. Any particular reason for excluding it?

Ah good catch, yeah I liked that one too. Added to this PR!

@pls148 pls148 enabled auto-merge (squash) March 4, 2025 20:59
@pls148 pls148 force-pushed the ps/rustfmt-nightly branch 4 times, most recently from 5ef6221 to c5875cf Compare March 5, 2025 01:37
@pls148 pls148 force-pushed the ps/rustfmt-nightly branch from c5875cf to ef7ee30 Compare March 5, 2025 05:08
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.

2 participants