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

Update Scalafmt to 2.4.2 #3337

Merged
merged 3 commits into from
Mar 1, 2020

Conversation

travisbrown
Copy link
Contributor

@travisbrown travisbrown commented Feb 29, 2020

This is a more conservative version of #3315 that still rewrites { } to ( ) for single-line expressions, but doesn't remove newlines in cases like this, which was my primary complaint about the default config in 2.4.2:

  forAll { (nes: NonEmptySet[Int]) =>
    nes.reduce should ===(nes.fold)
  }

In #3315 that got rewritten to the following, which obscures what's being tested, at least to my eyes:

  forAll((nes: NonEmptySet[Int]) => nes.reduce should ===(nes.fold))

Instead of +495 −1,432 it's +58, −50, so quite a bit less dramatic, and the changes look more reasonable to me.

@travisbrown travisbrown changed the title Update/scalafmt 2.4.2 Update Scalafmt to 2.4.2 Feb 29, 2020
@codecov-io
Copy link

codecov-io commented Feb 29, 2020

Codecov Report

Merging #3337 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3337      +/-   ##
==========================================
+ Coverage   93.31%   93.31%   +<.01%     
==========================================
  Files         378      378              
  Lines        7688     7689       +1     
  Branches      206      206              
==========================================
+ Hits         7174     7175       +1     
  Misses        514      514
Flag Coverage Δ
#scala_version_212 93.38% <100%> (ø) ⬆️
#scala_version_213 93.09% <100%> (ø) ⬆️
Impacted Files Coverage Δ
core/src/main/scala/cats/syntax/monadError.scala 80% <100%> (+2.22%) ⬆️
core/src/main/scala/cats/data/WriterT.scala 91.26% <100%> (ø) ⬆️
...eycats-core/src/main/scala/alleycats/std/map.scala 46.42% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Func.scala 96.42% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Cokleisli.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Kleisli.scala 90.82% <100%> (ø) ⬆️
core/src/main/scala/cats/instances/map.scala 95.91% <100%> (ø) ⬆️
core/src/main/scala/cats/instances/sortedMap.scala 86.15% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 482660e...81b7c50. Read the comment docs.

Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this format as well

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly don't like deviating from the defaults, but I'm persuaded on this one.

@travisbrown
Copy link
Contributor Author

Thanks all! We can always reconsider going back to defaults later.

@travisbrown travisbrown merged commit a8e2c21 into typelevel:master Mar 1, 2020
@travisbrown travisbrown added this to the 2.2.0-M1 milestone Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants