Skip to content

Issue #574 - Initial run of rustfmt on the whole codebase. #581

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

Closed
wants to merge 2 commits into from
Closed

Issue #574 - Initial run of rustfmt on the whole codebase. #581

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 28, 2017

Still 2 errors that rustfmt is not able to solve on its own.

I'll fix one by hand, and skip the other one since it's not possible to fix it automatically (it involves renaming a method of the OpenSSL library).

@ghost
Copy link
Author

ghost commented Feb 28, 2017

Wierd, by running cargo fmt again, it seems that some files didn't get formatted correctly at first.

@ghost
Copy link
Author

ghost commented Feb 28, 2017

There's an issue with rustfmt, the skip attribute doesn't seem to work properly. It will still yell at me for a method, the name of which is too long. I'll open an issue on rustfmt's repository. Maybe I use it badly, though.

@Eh2406 Eh2406 mentioned this pull request Mar 1, 2017
@carols10cents
Copy link
Member

@Insomgla let me know if you need help figuring out the rustfmt issue! Have you filed an issue with rustfmt yet?

I think I'm going to merge the ? change first, since it should be easier to run rustfmt again, hope you don't mind!!

@ghost
Copy link
Author

ghost commented Mar 1, 2017

That would be a smart choice indeed :p and I opened an issue, yes.

@carols10cents
Copy link
Member

Ah cool, found it: rust-lang/rustfmt#1345

@carols10cents
Copy link
Member

While not ideal, wdyt about setting rustfmt's max width to 138 (the length of this line)? That would let us move forward, and it's not much higher than the default of 100.

@carols10cents
Copy link
Member

Hi @Insomgla, since this has gotten a bit out of date and both crates.io and rust-fmt have changed in the meantime, I'm going to give this a close. I'm still interested in having rust-fmt enabled, please feel free to submit a new PR in the future and I won't let it sit so long next time, I promise! :)

@steveklabnik
Copy link
Member

Also, using https://github.com/rust-lang-nursery/rustfmt/blob/master/rfc-rustfmt.toml would be a good idea 💯

@elliotekj elliotekj mentioned this pull request Jun 12, 2017
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