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

Reformating via Scalafmt #898

Merged
merged 5 commits into from Jun 13, 2019

Conversation

Projects
None yet
4 participants
@alexandru
Copy link
Member

commented Jun 12, 2019

This PR is reformatting all files via Scalafmt. I've also tuned it a little.

Found fragments which I did not like, however the result is totally worth it, plus we can fix as we go along.

N.B. we already had a Scalafmt config, however if people actually used it on a source file, it would then commit reformatting changes too, which is very annoying.

Also if we let the project be driven by Scalafmt, we can include a test for formatting in our CI build.

alexandru added some commits Jun 12, 2019

@oleg-py
Copy link
Collaborator

left a comment

No sane person is going to review 837 changed files 😆

@Avasil
Copy link
Collaborator

left a comment

I don't see build.sbt in changed files - maybe we can enable scalafmt in compilation by default? So contributors don't have to do it themselves

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

@Avasil the Scalafmt documentation is recommending against it, as it slows down the compilation process and it messes with the IDE. You'll get a lot of "an external change has occurred, reload file?" issues?

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Alternative we could integrate a formatting check in test.

@alexandru alexandru changed the title Reformating via Scalafmt WIP: Reformating via Scalafmt Jun 13, 2019

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

@Avasil I just added scalafmtCheck in testing, to be executed by the CI.

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Not ideal to add more stuff to our build, we'll need to move to CircleCI.

@alexandru alexandru changed the title WIP: Reformating via Scalafmt Reformating via Scalafmt Jun 13, 2019

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Tests with scalafmtCheck pass 🙂 I'll assume you all agree 😛 I need to merge this in order to work relaxed on the other PR.

I think Scalafmt is doing a pretty good job for an automated tool, better then I expected, but as I said, we can fix it as we go along if we find fragments that aren't right.

@alexandru alexandru merged commit bc6b25c into monix:master Jun 13, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@xuwei-k

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.