https://pantsbuild.org/ logo
Join Slack
Powered by
Hey, I've been thinking more about "curse of knowl...
# development
h
Hey, I've been thinking more about "curse of knowledge" & making BUILD files more intuitive. (Including the assumption that many everyday pants users won't read our docs & we're Just Another Tool) I think the 
dependencies
field is a misnomer: it doesn't allow you to intuit dependency inference & how the field augments what gets inferred. Instead, I'm proposing renaming the field to 
extra_deps
 
explicit_deps
. That will hopefully prompt you to ask yourself "What are the not-explicit deps?" Changing this is is 100% automated for users by running 
./pants update-build-files
. We can easily have a long deprecation cycle, it's literally only 2 lines of code to support the old name. But ack that this does cause some churn, even though it's automated, like a bigger Git diff when upgrading.
➕ 3
w
are there anecdotes of people being confused by what that field means…?
h
From someone who's been using Pants for a long time: https://pantsbuild.slack.com/archives/C046T6T9U/p1642366845048200
Anecdotally, I think I have to make clear at least once a month when helping users w/ dep issues that the field augments And that's only from people we know on Slack, ~power users
w
it would be a bit weird for it to be named 
extra_deps
if dependency inference was turned off, or not implemented for a language
h
How often would you expect either of those to be the case?
or not implemented for a language
Tom made a good point in the new-language guide that dependency inference is the bread and butter of Pants, he recommended it being one of the first things to implement Also, the Target API lets plugin authors rename the field if they want, e.g. when adding a new language.
b
My thoughts: • 
dependencies
is verbose (although a very weak argument) • Setting 
dependencies
feels weird because I feel like I'm setting dependencies (not adding additional ones). E.g. it's not clear we're not setting and really we're merging • I'd be surprised if dep inference was off. That's like the #1 feature of Pants IMO. • I don't love 
extra_deps
because it kind of sounds like optional deps, but I like that it's short and isn't `deps`/`dependencies`. I'm happy with an equivalent
➕ 1
h
Other names to consider: 
explicit_deps
, 
additional_deps
➕ 1
f
It's still a bit confusing though in the light of target types that don't (and will never) support inference, like plain `target()`s. I guess 
explicit_deps
works in both senses though
➕ 1
h
That's a great point in favor of 
explicit_deps
. I don't love that 
explicit
is harder to type than 
extra
, but clarity is maybe worth it
f
And I think I was confused because it wasn't always like this? Like, when I first implemented a plugin in fall 2020, (Pants 2.1 maybe?) I think adding 
dependencies
overrode all inference
which honestly still makes more sense to me semantically
b
I wish we could unify with the options syntax 
deps.add
or 
deps
would be so perfect
h
In Pants 1.30, there was no dependency inference. So 
dependencies
made a lot of sense: that's all there was. Since Pants 2.0, it has always augmented inferred deps
f
okay maybe I was confused then too
😂 1
h
I think that's natural: it is not intuitive why 
dependencies=["foo"]
would be augmenting something. Unless you already know Pants has dependency inference & has this behavior, why the heck would you ever intuit something like that?
My hope with this proposal is that it not only helps new organizations trying out Pants, but also helps teams already using Pants when they have new team members join. When you join, there's already so much you have to learn not-related to Pants. We're Just Another Tool. So let's make it easy to intuit: don't assume everyone has read our docs
b
Eric, what if we mimic'd the options syntax, but with an underscore? Maybe more confusing but maybe not. 
deps
-> Sets the value and completely overrides 
deps_add
-> Adds the deps 
deps_remove
-> Removes inferred deps
b
I agree with Joshua that it seems hard to envision dep inference off as a common use case. I would have assumed it's toggleable pretty much only for debugging purposes. Have we had reports of it being commonly used for any other reason?
h
Have we had reports of it being commonly used for any other reason?
I think mostly only people migrating from v1 to v2 and wanting to minimize change. @flat-zoo-31952 you used to not use dep inference, right? But changed your mind?
Maybe more confusing but maybe not.
I think possibly confusing to users, and particularly hard to implement with the Target API. It's very field-centric whereby one field has all you need; spreading out one concept into 3 different highly related fields is not well supported. Also I can't imagine when you'd want to completely override? Seems like usually it'd only be 1-2 false positives you want to remove
f
No I always used it; I think I just needed it off for certain circumstances, because I was hacking to support different versions of tensorflow in the same repo or something
👍 1
I'd say for "turn it off here" a 
'!//::'
rule would work, right? Idk if you can use that "all targets recursively" syntax there though
b
Also I can't imagine when you'd want to completely override? Seems like usually it'd only be 1-2 false positives you want to remove
Actually this is something we haven't discussed. Adding 
"!..."
strings to 
explicit_deps
seems weird 😐
h
I'd say for "turn it off here" a '!//::'
yeah we could do something like that if the need ever arose; no need for a new field
f
Adding 
"!..."
strings to 
explicit_deps
seems weird 😐
Why? It's saying "explicitly not this"
➕ 1
🤔 1
I'd think about this a bit more, but I think this is generally a good idea and worth some churn. The more self-documenting Pants stuff is, the easier it is to sell and get buy-in from devs
➕ 1
☝️ 2
❤️ 1
b
I think it's worth reminding ourselves from time to time that Pants 2 is really a 1.x in some real ways. Having churn along the way to solidifying a really great user experience by the time we reach the next major release is to be expected and is a worthy goal imo.
➕ 2
h
I've been thinking about this tweet recently https://twitter.com/Jakeherringbone/status/1496904750469500928?s=20&t=rfSEr-BFZ3k4DYZ-Bgs5Zw. It's a balancing act: Pants does have a decent bit of churn....which has a real cost we should never forget. But not fixing things also has a cost. (Of course, the best is to get it right the first time hehe)
b
Yeah that tweet makes me sad.
f
I'm grateful that I don't have to be on twitter to learn Pants' best practices 😅
😂 2
b
This Slack is as close as I get to "social media". Coincidentally it's only Pants best practices
b
Whenever I develop a new project, whether it be a talk or app or knitting, the first version is a learning and evolutionary experience. Sure, ideally you'd get "it" right the very first time. That's worth aspiring to for sure. But expecting it is unrealistic and risks closing one off from taking in lessons from that first attempt. We care about listening to feedback, and sometimes the best way to do that is to put an imperfect solution out there to be tried, and then listen and observe to discover how to tweak from there. That's the approach being taken here. I don't think it indicates a problem. It indicates thoughtful deliberative process of improvement.
💯 1
➕ 1
https://pantsbuild.slack.com/archives/C0D7TNJHL/p1647545298185389?thread_ts=1647541244.444479&cid=C0D7TNJHL If you were a Bazel user, following the Bazel account wouldn't have led you to that info anyway. They only use it to promote BazelCon. It had exactly one tweet all year about anything else at all, and it wasn't that.
h
+1. I'd add that it's crucial to think about how painful the churn will be, and how to make it less painful. This thread would be a lot worse of an idea if we didn't have 
update-build-files
b
True
c
Also consider output like that from peek, where the inferred deps also are present..
(I’m kind of fond of the 
dependencies
field; but I haven't had to explain what it does to anyone either..)
f
Also consider output like that from peek, where the inferred deps also are present..
This is a good point too. But peek shows some things that are "computed" fields as well, like "address" and "target_type". Dependencies could be included in that
h
Note that 
peek
currently shows 
dependencies
vs 
dependencies_raw
, which is the actual field. So it would become 
dependencies
(everything) & 
explicit_dependencies
2 Views
Open in Slack
PreviousNext
Powered by