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 rustfmt.toml and use nightly formatting #49

Merged
merged 5 commits into from
Sep 13, 2022

Conversation

mattsse
Copy link
Contributor

@mattsse mattsse commented Sep 8, 2022

Add rustfmt.toml with some configs, listed here https://rust-lang.github.io/rustfmt/

use nightly formatting cargo +nightly fmt

add separate CI task for lining

1 clippy error, could either fix here or separately

Copy link
Owner

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

nice work!

I like breaking out the linting into a separate CI job.

I tend to prefer just the "stock" configuration for some tool to reduce the amount of overhead needed to operate a given codebase, e.g. requiring rustfmt.toml here although I'm open to these changes

I left some questions in the comments to address before merging. And about the CI failures, I'm fine to merge this in now if you want to follow up soon with another PR fixing the lints -- from what I can see we can just add some Boxes to the offending error variants and satisfy the lint

.github/workflows/rust.yml Show resolved Hide resolved
.github/workflows/rust.yml Outdated Show resolved Hide resolved
mev-boost-rs/src/relay_mux.rs Show resolved Hide resolved
@mattsse
Copy link
Contributor Author

mattsse commented Sep 9, 2022

made changes to the action

most of the rustfmt.toml settings concern line, comment wrapping, and removing trailing commas semicolons
some are a bit more opinionated, but imo really only

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

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

and

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

which allows more width for calls etc before it wraps

but happy to use the defaults instead

@ralexstokes
Copy link
Owner

ralexstokes commented Sep 11, 2022

im fine w/ the config you added, can you follow up with a PR to fix the lint?

if so ill go ahead and merge this in the mean time

@mattsse
Copy link
Contributor Author

mattsse commented Sep 11, 2022

fixed by boxing the large error variant

Copy link
Owner

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

excellent, thanks for this!

@ralexstokes ralexstokes merged commit 74f182a into ralexstokes:main Sep 13, 2022
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.

None yet

2 participants