Restore support for dependencies outputting 3rdparty deps through new enum option
#8960
Conversation
Eric-Arellano
requested review from
stuhood,
asherf,
cosmicexplorer and
TansyArron
| @@ -20,10 +27,16 @@ def register_options(cls, register): | |||
| super().register_options(register) | |||
| register( | |||
| '--transitive', | |||
| default=True, | |||
| default=False, | |||
There was a problem hiding this comment.
The V1 task will change its default to False in 1.27.0.dev0. I changed this one now to make sure we don't forget to also update the V2 implementation.
| lambda: not self.is_external_only and not self.is_internal_only, | ||
| removal_version="1.26.0.dev0", | ||
| lambda: opts.is_default("type") and not opts.internal_only and not opts.external_only, | ||
| removal_version="1.27.0.dev0", |
There was a problem hiding this comment.
I bumped these up one major version because Stu mentioned in a different PR that that's a good practice when we deprecate the prior solution to a deprecation that's still in flight.
Eric-Arellano
force-pushed
the
dependencies-external
branch
from
656cd03
to
469d9f1
Compare
| type=bool, | ||
| help='Run dependencies against transitive dependencies of targets specified on the command line.', | ||
| ) | ||
| # TODO(#8762): Wire this up. Currently we only support `internal`. | ||
| register( | ||
| '--type', type=DependencyType, default=DependencyType.INTERNAL, |
There was a problem hiding this comment.
Very open to suggestions for a better name. I couldn't think of anything more natural and generally like this, except that type is an overloaded word.
| @@ -11,6 +12,12 @@ | |||
| from pants.engine.selectors import Get | |||
|
|
|||
|
|
|||
| class DependencyType(Enum): | |||
| INTERNAL = "internal" | |||
There was a problem hiding this comment.
Internal and external are really unclear names to me, and I didn't know what they meant until I read your helptext on the flag. How would you feel about source and 3rdparty?
There was a problem hiding this comment.
Great recommendations! Thank you!
Thanks Daniel!
Eric-Arellano
force-pushed
the
dependencies-external
branch
from
469d9f1
to
8e5512f
Compare
Eric-Arellano
changed the title
dependencies outputting external deps through new enum optiondependencies outputting 3rdparty deps through new enum option
Turns out, there is a use case for
./pants dependencies --external-only, such as scripts using it to determine the exact version strings of Python requirements used by Pants targets.However, the option names are not ideal, such as having to enforce that they're mutually exclusive at runtime. It's also a bit confusing to default to
internal and external. Ideally, we'd default tosources(akainternal) and only include3rdparty(akaexternal) when requested.We can make both improvements through a new enum option, which wasn't available when we first wrote this due to Python 2 not having enums.