https://pantsbuild.org/ logo
Join Slack
Powered by
I found an awkward UX bug: ```▶ ./pants test src/...
# development
h
I found an awkward UX bug:
Copy code
▶ ./pants test src/python/pants/util/strutil_test.py src/python/pants/util:tests
...
✓ src/python/pants/util/strutil_test.py:tests succeeded.
✓ src/python/pants/util/strutil_test.py:tests succeeded.
That is, the result shows up twice for the same file address. That’s because 
TargetRootsToFieldSets
stores a mapping of 
TargetsWithOrigin
to 
FieldSet
, and this is the same target but two distinct origins The fix is simple for this if we can get rid of 
TargetsWithOrigin
et al. We no longer use that mechanism anymore, and it has some weight to support. What do we think about shedding the ability to use 
WithOrigin
? In the past, we used it for globbing semantics, to only error if a certain type of spec was used. Since then, we’ve made globbing semantics more uniform to never error and only ever run on things we are able to
cc @happy-kitchen-89482 if you’re fine with removing 
WithOrigin
w
is this “removing” it, or this “using WithOrigin in fewer places: only those that need origins”?
h
To fix this bug, we only need to lose the 
targets_with_origins
property from 
TargetRootsToFieldSets
. But imo, if we want to keep 
WithOrigins
in general, we should probably have it there too Right now we have: * 
AddressesWithOrigins
* 
TargetsWithOrigins
* 
TargetRootsToFieldSets.targets_with_rigins
My two cents: 1. It’s good to delete code like this. We have a lot of abstractions already, and this will simplify things a bit. Also, less wasted work for users; these rules always run when resolving specs 2. It used to be a helpful mechanism, but our UX design has evolved to where we are now. It’s no longer relevant. 3. If a plugin author needs it in the future, they can add back the logic
4. we didn’t deem this mechanism worthy enough to document at https://www.pantsbuild.org/v2.1/docs/rules-api-and-target-api
w
yea, if the existing usecases don’t meet the bar (or there aren’t any) then deleting code is great.
h
The only use case I’ve found is to echo back to the user what specs they ran with, but we can solve that another way
Deleting WithOrigins is a big change, and we want to cherry-pick..I’ll split out fixing this UX bug vs. the overall change
w
hm. do we really need to cherry-pick?
h
True
w
it’s not ideal, but it also seems ok to fix forward.
h
Yeah it doesn’t break anything, only is confusing UX
2 Views
Open in Slack
PreviousNext
Powered by