Remove `rustfmt` from travis builds

[mcarton]

I vote to remove rustfmt from travis.

I find it really annoying, blindly applying rustfmt suggestions does not make the code clearer in lots of instances and even often make it worst (recent example).
It also seems really hard to make rustfmt happy both locally and on travis (1, 2, 3).

[oli-obk]

we can set it up so that rustfmt is required to be run once a month. That way we won't diverge much and it doesn't influence day to day business.

[Manishearth]

I'm okay with the suboptimal suggestions, since the vast majority of suggestions are good, and the suboptimal ones are subjective.

The local/travis thing is a version issue.

That way we won't diverge much

It tends to diverge quickly IME. We might have better experiences here though.

[killercup]

I think the idea of running rustfmt on travis is good. But you should definitely pin it to a specific version to make it more deterministic.

Also, while you are at it, please consider using block ident for function arguments, which reduces line length as well as diff churn. rustfmt has fn_arg_indent = "Tabbed" but sadly it doesn't seem to change to current visual indented code in any way…

[llogiq]

Perhaps running it (to inform rustfmt devs) without failing the build?

[oli-obk]

It's possible to set up travis targets that don't fail the build, but they won't show up red in the pr, just in travis.

[killercup]

If it doesn't fail the build, nobody's gonna notice. Unless you manage to write a script that automatically comments on the PR with the rustfmt diff. (Just like the one I want for compiler warnings.)

[llogiq]

True.

[Manishearth]

You can set up a CircleCI build or something, so you get a separate indicator. Gate merges on travis only.

…

-Manish Goregaokar

On Sat, Jan 28, 2017 at 9:21 AM, llogiq ***@***.***> wrote: True. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/Manishearth/rust-clippy/issues/1487#issuecomment-275861107>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSMLdOXwiA5eYJG9TnSgzNKvSMn-Qks5rW3kEgaJpZM4LwgU7> .

[oli-obk]

Alternatively we set up a travis job that only does rustfmt checks and use something like https://github.com/wizeline/travieso#heroku , but I don't have a heroku or other server to host it on.

Or we wait until travis-ci/travis-tasks#78 is merged to automatically get one icon per github job.

Or we just remove the rustfmt checks and add a travis job on master commits that automatically runs rustfmt and pushes a commit to the master branch if rustfmt changed anything.

[oli-obk]

A great argument from @dtolnay against rustfmt as a blocker or even showing up in travis:

I don't think that would be valuable. The tradeoff is scaring away new contributors. If someone makes a PR, I would prefer that they remember it as a reasonable discussion about meaningful things, rather than as a struggle to satisfy a grumpy buildbot. Even if the overall build is green, seeing a red task can be anxiety-inducing and uncertain about whether you are responsible for fixing it. Instead, if the code looks mostly fine then I won't notice or care anyway, and somebody will run rustfmt every month or so and it takes 10 seconds to clean everything up so there isn't really anything to gain from enforcing it.

[oli-obk]

Although I feel like the same argument could be made for util/update_lints.py, it's not like it's very important for that list to be 100% up to date. We can make rustfmt and update_lints a prerequisite of version bumps instead.

[mcarton]

👍 We've already had people complaining that they can't use a lint described in the README because it was not published yet.