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

scalafmt format #87

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

scalafmt format #87

wants to merge 2 commits into from

Conversation

carlosms
Copy link
Contributor

@carlosms carlosms commented Apr 6, 2018

Partially replaces #74, see #74 (comment).

This branch was created from #86, the relevant commit is just bd39545.

carlosms added 2 commits April 6, 2018 18:33
Adds make lint-scala command, used by travis

Signed-off-by: Carlos Martín <[email protected]>
@carlosms carlosms requested a review from bzz April 6, 2018 16:46
@bzz
Copy link
Contributor

bzz commented Apr 9, 2018

Looks great, let's finish and merge the linter first and then get back and :shipit:

@smola
Copy link

smola commented Apr 12, 2018

Before merging scalafmt and perform any reformatting, we should discuss if we would like to do for default or intellij styles. I have also seen that the following might need to be added to enforce line length:

# disable case arrow alignment, it breaks line length
align.tokens = []

It would be desirable if:

  • Our .scalafmt.conf does not conflict with previous scalastyle-config.xml (actually, both might be kept, since scalastyle checks for stuff scalafmt doesn't.
  • It produces minimal reformatting on existing code (this might not be possible, but...).
  • We can push this company-wide for Scala projects.

@smola
Copy link

smola commented Apr 12, 2018

I discussed this offline with @bzz. Just to be clear: I don't want to block this, but if you plan to produce massive reformatting with it (e.g. >1k lines across multiple files), let's double check with others.

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.

3 participants