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

bitter-ability-32190

03/02/2022, 1:08 AM
Cache boost idea for formatters when linting ๐Ÿงต It should be possible to have both
fmt
and
lint
use the same process (the args used for formatting) for both
fmt
and
lint
.
fmt
then applies the changes to the workspace while
lint
diffs (optionally outputting the diff if requested). Then
./pants fmt lint ::
is a no-op in
lint
for formatters ๐ŸŽ‰ The risk is meaningful
stdout/err
being removed from the tool, but the gain is perf through caching, consistency, and plugin simplification.
@witty-crayon-22786 who always has insights into these kinda things ๐Ÿ™‚
h

hundreds-father-404

03/02/2022, 1:13 AM
I'm generally +1 on this proposal, but do worry about the UX of this:
Copy code
โฏ ./pants lint --only=black ::            
18:12:04.56 [WARN] Completed: Lint with Black - black failed
reformatted src/python/pants/goal/stats_aggregator.py

All done! :sparkles: :cake: :sparkles:
1 file reformatted.
โž• 1
b

bitter-ability-32190

03/02/2022, 1:15 AM
I might be in the minority, but I think the formatters
stdout/err
is meaningless (unless they crash-and-burn). All you are told is a verbal yes/no. but each tool does so slightly differently. If anything, I think if Pants took the reigns so-to-speak UX would improve due to consistency.
h

hundreds-father-404

03/02/2022, 1:18 AM
If anything, I think if Pants took the reigns so-to-speak UX would improve due to consistency.
Something fascinating about your proposal is that it makes it more feasible for Pants to list the files that changed. Iirc docformatter doesn't tell you what failed, only that something did. Which is annoying if you want to run on just 1 file rather than 2000 the next run. Parsing std{out,err} would not work, but diffing the input vs output digest could. I'm still pretty hesitant to move into the brave new world where Pants is writing std{out,err} for you because it seems like so many unknowns, like if you really really want Black's output because you're debugging something. But this makes me slightly more open to it
โž• 1
b

bitter-ability-32190

03/02/2022, 1:19 AM
Obligatory
experimental-...
๐Ÿ˜›
h

hundreds-father-404

03/02/2022, 1:21 AM
Which is fine temporarily, but I do think we want a clear vision for where we're going w/ it. I worry about options fatigue (as someone who adds a lot of options hehe). It's more code for us to maintain and that might break, and it's more stuff to document and for users to discover Altho, feasible if the plan is "let's have this as experimental for 1 release to collect feedback, then decide the direction we want to commit to." ๐Ÿ™‚
โž• 1
w

witty-crayon-22786

03/02/2022, 1:23 AM
I continue to be fine removing the output (by default) of linters/fixers when they succeed. I think that rendering only "these files were changed" (or the diff) might even be a better experience
๐Ÿ‘ 2
๐Ÿ™ 1
h

hundreds-father-404

03/02/2022, 1:24 AM
Given how many formatters/linters we have, I become more and more open to that direction. I wrote this in our 2.10 release blog lol
A major benefit of Pants is that it gives you a consistent and single interface for running all your linters and formatters, regardless of the language:
Copy code
โฏ ./pants lint ::
โ€ฆ
โœ“ autoflake succeeded.
โœ“ black succeeded.
โœ“ docformatter succeeded.
โœ“ flake8 succeeded.
โœ“ gofmt succeeded.
โœ“ google-java-format succeeded.
โœ“ isort succeeded.
โœ“ shellcheck succeeded.
โœ“ shfmt succeeded.
especially w/ batching of tools, that is a lot of output now
b

bitter-ability-32190

03/02/2022, 1:25 AM
Relatedly I think simplifying the plugin interface for formatters opens the door for more ๐Ÿ˜ˆ
โž• 2
h

hundreds-father-404

03/02/2022, 1:27 AM
Something fascinating about your proposal is that it makes it more feasible for Pants to list the files that changed.
Yeah I like this a lot. I remember @fancy-motherboard-24956 first proposed this consistent interface you're talking about way back in 2019, and a reason we didn't go with it is how it would be a lot of maintenance burden to try to parse the std{out,err} of every single tool, especially because users can change the
--version
. We'd have to teach Pants what each tool's output looks like - no good This is instead highly generalizable ๐Ÿ™Œ
๐Ÿ™Œ 1
Timing update: Overall I don't see a wall-clock timing difference before/after. I suspect this has to do with the fact that formatters are already very fast and linters are comparatively slow. I do see a noticeable CPU time difference though. I still believe that the UX improvements and the reduced plugin boilerplate is well worth the change.
๐Ÿ‘ 1
h

hundreds-father-404

03/03/2022, 4:59 PM
Ohhhhhh Joshua I bet your overpowered machine is biting you! Try lowering concurrency to 2-4 cores, which is much more typical for users
b

bitter-ability-32190

03/03/2022, 5:10 PM
Trying with 4
h

hundreds-father-404

03/05/2022, 1:34 AM
Joshua, your Google Doc is a +1 from me. I like the UX proposal after sitting with it for a few days. My only lingering concern is how to handle dumping the original formatter output if users want it. I like your
-ldebug
proposal, but am not sure how to get it to work well. I recommend pinging more people on Monday to discuss / sign off, then you could proceed with implementation ๐Ÿ™‚ (Also where did the benchmarks go in the doc?)
๐Ÿ™Œ 1
โœ… 1
b

bitter-ability-32190

03/05/2022, 1:42 AM
(Also where did the benchmarks go in the doc?)
Scroll down?
๐Ÿ‘€ 1
h

hundreds-father-404

03/05/2022, 1:43 AM
oh long day, page break threw me off
b

bitter-ability-32190

03/05/2022, 1:51 AM
I had to be judicial or the formatting get funny