https://pantsbuild.org/ logo
Join Slack
Powered by
Does anyone know why <https://github.com/pantsbuil...
# general
a
Does anyone know why https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/project_info/tasks/idea_plugin_gen.py uses 
self.context.options.positional_args
rather than 
[target.address.reference() for target in self.context.target_roots()]
or similar?
w
iirc, that was to allow it to capture and pass the CLI specs to the IDEA plugin
so that when it loaded/reloaded, it would get the latest/complete set of targets
if that doesn't sound right, can ping yi
a
As in, to allow 
::
to continue to expand if new targets are added?
(This came up because it means that 
--owner-of
doesn’t work for this goal)
w
yep
...mm.
best laid plans.
could possibly add a conditional there...? bummer though.
really, "the roots" are defined by one of three things. reifying that up into a single enum somewhere to make it more obvious would be good. i think Danny did a bit of that with --query
a
We could probably try to do a merging? Merge in extra specs from 
target_roots()
which aren’t covered by a spec from the list of positional args?
w
mm, maybe? an issue might be that if you include concrete targets and they get deleted later the export will start failing.
cc @damp-easter-17791
a
Doesn’t seem hugely different to the failure modes of the status quo
w
hm?
...i suppose that we ask people to reload any time a BUILD file changes anyway.
yea. ok
would still probably rather do an "either/or"
oh, wait. no
the issue is that if your project was configured with a deleted target, reloading will not work
and you'd need to throw it away and re-import.
(if specs, use them, otherwise use roots)
a
Sure, but whether that happens because I ran: 
./pants --owner-of=foo/bar idea-plugin
or 
./pants idea-plugin foo:bar
we have the same problem, right?
It’s only if I ran 
./pants idea-plugin foo:
that I wouldn’t error
w
the issue is that you can't distinguish the first two cases
...or whatever
a
Sure, but they should be identical, and right now only one of them works?
w
...other than by seeing whether there are specs
if there are specs, use them
@average-vr-56795: right, i'm not saying don't fix the issue. i'm saying: the fix is probably a conditional of "use specs if we have them, otherwise target roots"
a
I’m saying: Expand the specs If there are any roots which aren’t covered by a spec, add that single-target spec to the list of specs Use the merged list
w
that isn't possible, because you may only use exactly one of 1) specs, 2) --owner-of, 3) --changed
a
I guess we could say “If there are both specs and target roots, and they’re not effectively identical, error”
w
a
Oh! I was not aware! Ok, yes, I’m with you, a conditional sounds… Well, not great, because ideally a task shouldn’t have to know that, but like the right solution here 🙂
w
yea. this should be converted into an enum that makes the either-or-ness more obvious. see my comment about Danny/--query
a
That makes a lot more sense now 🙂 Cool!
w
other thing is that Yi has suggested that we should definitely port this to v2
which i agree on, but which (for exactly this reason) would probably involved doing the enum thing
6 Views
Open in Slack
PreviousNext
Powered by