Enforce code style with rustfmt

[SUPERCILEX]

Is this a desirable change? Or maybe a better question: is there a reason this hasn't been done before?

[rtzoeller]

I'd like to see it done at some point, but there's a few reasons I haven't pushed for it yet.

1. I'd also want an enforcement mechanism to ensure we don't regress.
2. It immediately makes almost every outstanding branch have merge conflicts.

[SUPERCILEX]

Okey dokes, that makes sense. Let's kill two birds with one stone then: fmt is run on every change and it only runs on the changed files, allowing for an incremental migration. This kind of sucks during the migration period since PRs will include extraneous changes, but I can't think of any better way to do it without completely ripping off the bandaid and breaking the 70 open PRs.

It works: success on no changed files and failure on changed files.

[rtzoeller]

Incidentally I think this approach will still break every open PR, since their builds will fail with formatting changes unless the files they modify are already perfect. Without rustfmt supporting checking only modified lines (i.e. git-specific integration), I don't see a way to truly do this incrementally.

My preference would be that we get the next release out the door (0.24.0) and work through the backlog of mergeable and trivially rebase-able PRs, and then revisit turning on rustfmt. I think formatting the entire repo in one pass is the correct approach, to make it easier to use --ignore-rev/--ignore-revs-file, but I don't think now is the best time for it.

@asomers what are your thoughts?

[SUPERCILEX]

Works for me. I'll let you rip the bandaid and then you can merge this to add enforcement.

[asomers]

Incidentally I think this approach will still break every open PR, since their builds will fail with formatting changes unless the files they modify are already perfect. Without rustfmt supporting checking only modified lines (i.e. git-specific integration), I don't see a way to truly do this incrementally.

My preference would be that we get the next release out the door (0.24.0) and work through the backlog of mergeable and trivially rebase-able PRs, and then revisit turning on rustfmt. I think formatting the entire repo in one pass is the correct approach, to make it easier to use --ignore-rev/--ignore-revs-file, but I don't think now is the best time for it.

@asomers what are your thoughts?

I agree exactly.

[pacak]

Any chances to get this merged? While it will break any non formatted pull request, fixing it is as simple as formatting it again. With editor configured to auto format rust on safe working on non formatted files is very unpleasant.

[costinsin]

Should be closed by #1748

[rtzoeller]

Closed by #1748