Question: Is there any particular reason scalafmt ...
# generalf
fancy-motherboard-24956
03/04/2020, 2:31 PMQuestion: 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 PMHow often do we touch that Scala code? My objection would be performance, that now
githooks/pre-commitfmta
aloof-angle-91616
03/04/2020, 2:49 PMscalafmt is very fast with graal
native-imagealoof-angle-91616
03/04/2020, 2:49 PMthere is very little remaining work to merge that PR
f
fancy-motherboard-24956
03/04/2020, 2:49 PMI’m guessing the pants overhead is the main problem here?
a
aloof-angle-91616
03/04/2020, 2:49 PM👍 1
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 PMit would be a really nice small win though if we're looking for excuses to make inroads into the v2 jvm backend
➕ 2
aloof-angle-91616
03/04/2020, 2:50 PMwhich 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.
hundreds-father-404
03/04/2020, 2:55 PMI 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 PMNote 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 PMThat 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!
hundreds-father-404
03/04/2020, 3:04 PMAnother 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 PMYes, that sounds better.
❤️ 1
fancy-motherboard-24956
03/04/2020, 3:08 PMBest of both worlds as git is pretty responsive in the pants repo
6 Views