https://pantsbuild.org/ logo
Join Slack
Powered by
interested in thoughts on how we can reasonably de...
# development
w
interested in thoughts on how we can reasonably deprecate the behaviour that 
./pants list
will "fully validate*" the graph that you run it on (*which is not quite true, but which is less true in v2 than in v1): https://github.com/pantsbuild/pants/pull/6880#discussion_r275732169
i can only really imagine adding a flag that warns with a big explanation of the behaviour change when it is not set, and then deprecating it in a version or two...? but that would be... annoying.
cc @average-vr-56795
cc @happy-kitchen-89482
a
we can do delayed deprecations with https://github.com/pantsbuild/pants/pull/7494, and that's p much exactly what it was intended for i think (i wanted to use it for the 
--shlexed-build-and-test-flags
for the go test task, but didn't get it in in time)
👍 1
a
Deprecating list and renaming it to something with a frustratingly less suitable name?
(it's not a good answer...)
w
Mm. Yea, I think that temporary option feels more friendly, even though it would be annoying.
a
it might be worth adding special support for temporary options on top of the delayed deprecation period at some point if this pattern works out -- at some point
w
--i-acknowledge-that-the-easiest-way-to-speed-something-up-is-to-do-less-work
👌 1
h
I think the flag idea, while clunky, is totally reasonable.
w
...mm. it occurs to me that we should probably not do a bunch of these deprecations... and that if we were going to do just one of them, we'd want to have something relatively concrete to recommend that people do instead. i think that that is something like suggesting the combination of 
--changed
and 
test
a
we should probably not do a bunch of these deprecations.
why is this, specifically? adding the option which has a big fat warning seems like it allows naming a concrete thing to do instead -- if we made that warning link to the CI docs then that seems highly concrete
w
what i meant was: we should not do a deprecation cycle for every goal we migrate
particularly if what we're notifying people of is a particular CI model
a
I don't think 
test
is necessarily a suitable replacement here... I'm kind of tempted to add a 
validate-target-args
goal... For now, it could just construct a v1 
BuildGraph
but as we hone the target API that could be replaced by type checking args (including dep types)
w
validate-target-args
could not validate what all of the various tasks would consume from the target though.
(i don't know if that is making perfect the enemy of the good, but: only actually running 
test
will tell you whether you have specified the arguments that 
test
consumes correctly, etc)
a
For sure 
test
is the most correct answer here, but it's really, really expensive...
w
it's exactly what folks already do in CI...
but one important detail is that 
--changed-include-dependees=...
does validate dependency edges
a
I thought at Twitter we did a "make sure your build files are ok" step before we kicked off a test...
w
that's the 
--changed
portion
a
Hmm... Maybe I'm trying to solve a problem that isn't worth solving...
w
yea, i'm not sure either.
so, for CI in particular, there is an interesting distinction between "finding dependees" and "validating affected targets"
assuming that the first step finds everything (...that still exists: just remembered that this is one of the rubs: deleting BUILD files is problematic), then the second step accomplishes all individual-target validation
a
I think the place I care about this goal is more around local changes ("I've just refactored a bunch of build files, are my targets in an ok enough state that I can kick off CI and go to lunch while it runs, or is it going to immediately fail because I have a typo in their word `dependencies`")
w
that's all happening remotely anyway though. unless you're talking about validating all of that pre-submit
a
Yeah, I mean before I push
w
hm. well 
lint
finds all(/most...) of that
a
Hmm... Maybe that's quick enough to do the job...
w
i'll make https://github.com/pantsbuild/pants/issues/7581 a blocker for the list as 
@console_rule
change.
...if we're assuming that folks are already using 
--changed
, I wonder if we even need the noisy deprecated option
because 7581 has nothing in particular to do with 
list
, and everything to do with ensuring that the dependees of a deleted file actually end up parsed.
Open in Slack
PreviousNext
Powered by