https://pantsbuild.org/ logo
#general
Title
# general
f

fancy-motherboard-24956

03/04/2020, 2:31 PM
Question: Is there any particular reason scalafmt isn’t enabled by default in pants’ own config? We do have some scala code. If I enable it in pants.ini and format all our scala code, will anyone have any objection?
h

hundreds-father-404

03/04/2020, 2:36 PM
How often do we touch that Scala code? My objection would be performance, that now
githooks/pre-commit
(which is already slow) has to run V1
fmt
over Scala Otherwise, Scalafmt is a great tool. Foursquare loves it
a

aloof-angle-91616

03/04/2020, 2:49 PM
scalafmt is very fast with graal
native-image
, see https://github.com/pantsbuild/pants/pull/8772
there is very little remaining work to merge that PR
f

fancy-motherboard-24956

03/04/2020, 2:49 PM
I’m guessing the pants overhead is the main problem here?
a

aloof-angle-91616

03/04/2020, 2:49 PM
f

fancy-motherboard-24956

03/04/2020, 2:49 PM
(We don’t have a lot of scala code to fmt anyway; and most commits should not touch it, so not format it?)
a

aloof-angle-91616

03/04/2020, 2:50 PM
it would be a really nice small win though if we're looking for excuses to make inroads into the v2 jvm backend
2
which i am
h

hundreds-father-404

03/04/2020, 2:53 PM
(We don’t have a lot of scala code to fmt anyway; and most commits should not touch it, so not format it?)
We’re soon going to be able to stop using V1 lint and fmt. We already use exclusively V2 formatters, and are close to replacing Python checkstyle with Flake8 The only remaining linter would be MyPy, which Toolchain has a V2 implementation of but performance isn’t great so we haven’t yet upstreamed. (Maybe we should upstream for more V2 dogfooding though?) Once that happens, it would be nice to avoid ever using V1 in the pre-commit hook.
I definitely won’t block on this and think scalafmt is great, only want to raise it as a concern, given how little Scala code we have. Maybe my fears are unfounded, though, due to target selection! Do you have a chance to informally benchmark how much it would slow down the pre commit hook? Notably, we use —changed-since to avoid running on unmodified targets. That should help
f

fancy-motherboard-24956

03/04/2020, 2:59 PM
Note that alternatively we could avoid running fmt v1 in the pre-commit hook but run it on CI. That would mean faster iteration most time and longer iteration when touching scala which is rarer. (assuming the slow down to the pre-commit hook is indeed a problem)
h

hundreds-father-404

03/04/2020, 3:03 PM
That would make me feel much better! I’d be very happy with using Scalafmt if we only added it to the lint CI shard and not the pre-commit hook!
Another possibility is to conditionally run it in the hook iff Scala files changed. Notably, we only use rustfmt when Rust has changed. I think you could use git to quickly see if any files changed end in .scala for very low overhead
1
f

fancy-motherboard-24956

03/04/2020, 3:08 PM
Yes, that sounds better.
❤️ 1
Best of both worlds as git is pretty responsive in the pants repo
6 Views