https://pantsbuild.org/ logo
Join Slack
Powered by
I'm looking at two improvements and think we can c...
# development
b
I'm looking at two improvements and think we can combine them if we change the surface of 
fmt
and 
lint
. Collecting thoughts... 1. Support for targetless-formatters a. This is tricky because unlike 
lint
, we can't run a formatter on a file in parallel with another formatter on that same file. 2. Simplification of formatters (and soon to be fixers) in lint a. This is important because not only are we getting it wrong, but the complexity makes future modifications scarier The solution: • Make the 
fmt
and 
lint
types share a base interface (
lint
-based types) so `lint`'s implementation can be relatively 
fmt
-agnostic • "unify" both target and targetless tools by turning this into a 2-step process: a. Given a set of specs, ask the tool to filter/batch the inputs it cares about. Tools can attach metadata to batches (like partition info) b. Then issue requests using the batch (the Snapshot) I hate that we're changing the shape of `fmt`/`lint` plugins in most releases, BUT I do think we are converging on something generic enough to simultaneously cover most bases and still keep it simple. NOTE: There is no User-facing change here. Only internal and external plugin changes.
w
• Make the 
fmt
and 
lint
types share a base interface (
lint
-based types) so `lint`’s implementation can be relatively 
fmt
-agnostic
fwiw: they both extend 
StyleRequest
currently
• “unify” both target and targetless tools by turning this into a 2-step process:
this is already a two-step process afaict… it’s just that the “matching” is via FieldSet rather than, say, a filename pattern
but yea, seems reasonable overall.
b
I'll try and sketch it out more in-repo then we can discuss on pr
w
...oh, actually one other comment: ideally the plugins would not choose batches, as that is a lot of extra work for them i think? if they just match, and then the caller chooses batches, that centralizes the logic more.
b
Ah here a batch should've been called "partition"
👍 1
w
still… ideally that would be driven by the caller based on particular FieldSet values, for example (e.g. 
lint
batches all targets with a Field value of 
a
), rather than via imperative code
one of the thoughts i’d had was that if you had computed Fields (i.e. actual native 
AsyncField
, perhaps via the 
@union
-method-
@rule_helper
thing), then it would be even more useful for `lint`/`fmt` to partition based on field values
b
I agree that'd be ideal. I don't quite grok all the union shenanigans though.
w
but yea. in the absence of 
@union
-method-`@rule_helper`s like that, having two callsites for the 
@union
is essentially equivalent.
i.e.: 
await Get(Thing1, LintTargetsRequest(..))
await Get(Thing2, LintTargetsRequest(..))
b
I think while I'm thinking about ideals. A "fallback" implementation as a rule would be very useful
But perhaps the rule methods would fix
For now I think I'm going to mimic `Target`'s dynamic plugin field union shenanigans to get this to work 🙂
Open in Slack
PreviousNext
Powered by