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

cargo fmt with stable #876

Merged
merged 1 commit into from
Mar 21, 2023
Merged

cargo fmt with stable #876

merged 1 commit into from
Mar 21, 2023

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Mar 21, 2023

Just using the stable defaults, because I don't like having to remember (and remind others) to use cargo +nightly fmt. IMO in general I prefer this. Possibly vertical imports are nicer to slightly reduce the risk of merge conflicts in that area, but I'm not super fussed. What do you guys think? Any rules you want to see added?

@jsdw jsdw requested a review from a team as a code owner March 21, 2023 15:46
@jsdw jsdw changed the title cargo fmt with stable defaults cargo fmt with stable Mar 21, 2023
Copy link
Collaborator

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

There are a few tradeoffs we'll make here in terms of readability, although nothing unreasonable.

As you mentioned, the vertical imports were a bit clearer to read. Another aspect is that the stable tends to keep things closer to one-liner:

// unstable
TypePathType::Primitive { def } => {
                syn::Type::Path(match def {

->

TypePathType::Primitive { def } => syn::Type::Path(match def {

I generally think that using stable is a good idea, at least to not worry about deprecated config flags. LGTM! 👍

@jsdw
Copy link
Collaborator Author

jsdw commented Mar 21, 2023

I assume a bunch of the one-liner-isations are because it has 100 chars rather than 90 to play with now in terms of line length, though I didn't pay very close attention to the existing rules so perhaps some others also preferred splits!

I'm easy either way really; as long as it's stable I'm open to any rules and config

Copy link
Member

@niklasad1 niklasad1 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 all good with "rustfmt stable" and I like the imports better but also keep in mind that I'm not that modern :)

@niklasad1 niklasad1 requested a review from ascjones March 21, 2023 16:25
@jsdw jsdw merged commit 7c252fc into master Mar 21, 2023
@jsdw jsdw deleted the jsdw-stable-fmt branch March 21, 2023 16:53
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.

3 participants