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

Merged
merged 1 commit into from
Mar 8, 2025
Merged

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 11 times, most recently from 9e5ce8d to b328f92 Compare March 7, 2025 02:36
sveitser pushed a commit that referenced this pull request Mar 7, 2025
* remove boxedsync recv

* add comment back
@pls148 pls148 force-pushed the ps/rustfmt-nightly branch from 1c06de1 to c1eafb2 Compare March 8, 2025 21:30
@pls148 pls148 merged commit 984bcf5 into main Mar 8, 2025
50 of 52 checks passed
@pls148 pls148 deleted the ps/rustfmt-nightly branch March 8, 2025 22:50
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