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 to run on travis and fail the build #575

Closed
carols10cents opened this issue Feb 24, 2017 · 8 comments
Closed

Add rustfmt to run on travis and fail the build #575

carols10cents opened this issue Feb 24, 2017 · 8 comments

Comments

@carols10cents
Copy link
Member

After #574 is done, so that we have one commit with all the old code reformatted, I'd like to keep the code formatted nicely going forward by adding rustfmt to Travis like we have jslint that fails the build if the javascript isn't formatted correctly.

So this bug entails:

@ishitatsuyuki
Copy link
Contributor

rustfmt has awful side effects. Clippy deprecated the approach: https://github.com/Manishearth/rust-clippy/issues/1487

@carols10cents
Copy link
Member Author

@ishitatsuyuki Are you talking about the contributor-scaring-away side effects? We already have jslint that fails the build if the javascript isn't formatted right, and I have seen PRs that contributors have had the jslint check fail and then they fix it and push another commit, which hasn't seemed to be a deterrent to contributing. I'd like to try, and I'll be here to help people understand...

If those aren't the awful side effects you're talking about, could you elaborate please? Thanks!

@ishitatsuyuki
Copy link
Contributor

rustfmt could generate wrong, or unintended ugly output. See the clippy issue for details.

@carols10cents
Copy link
Member Author

Wrong output sounds like a bug in rustfmt, and more users of rustfmt would help find and fix these bugs.

Ugly is in the eye of the beholder; what I'm trying to solve here is consistent code formatting.

Unintended sounds like something we could add a rustfmt ignore annotation to.

I don't see anything egregious in the issues linked in that clippy issue, I remain unconvinced that we shouldn't even try.

@ishitatsuyuki
Copy link
Contributor

At least, we would get a failure even when the code is not wrong (only formatting problems), and that looks bad. Clippy's PR list was filled with failed statuses before, which was caused by probably broken formatting in master.

@vignesh-sankaran
Copy link
Contributor

Hey, is this still an issue that is still blocked? If so, what's the block?

@carols10cents
Copy link
Member Author

I forget why I added the blocked label, tbh!

@carols10cents
Copy link
Member Author

Closed by #771!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants