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

restyle to match dropshot, omicron, plus general style clean-up #35

Merged
merged 4 commits into from
Aug 4, 2022

Conversation

davepacheco
Copy link
Collaborator

This change does a one-time application of the following rustfmt changes:

normalize_comments = true
format_strings = true
wrap_comments = true
comment_width = 80

This is just to switch to C-style comments and rewrap strings and comments at 80 columns. I did these in separate commits so we could evaluate the options separately in case we don't like them. @andrewjstone what do you think?

We don't want to actually update rustfmt.toml for all the reasons discussed in oxidecomputer/dropshot#77.

If anybody has an outstanding change that they want to sync with this, I believe the thing to do is: immediately before sync'ing up with this change, commit any local changes, copy the above config lines (plus unstable_features = true) into your rustfmt.toml by hand, run cargo +nightly fmt to restyle your clone, remove the local changes to rustfmt.toml, commit the restyled version, and then sync up. I believe if you do this you will avoid conflicts from this change.

@davepacheco davepacheco marked this pull request as ready for review August 4, 2022 02:56
Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

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

This all looks good to me. Thanks @davepacheco.

If I understand correctly, we don't want to check in the rustfmt.toml changes because they rely on unstable which can change formatting on commit, right?

@davepacheco
Copy link
Collaborator Author

That is a big reason, yes. It was also rather a pain because you have to use nightly. New contributors have to set up their editor and it’s easy to forget the +nightly when you run by hand. If you forget to use nightly, it fails badly: just totally reformats things ignoring the unstable options.

@davepacheco davepacheco merged commit af778d6 into main Aug 4, 2022
@davepacheco davepacheco deleted the restyle branch August 4, 2022 15:30
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