New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some deprecated pex-related functions. #6865

merged 1 commit into from Dec 5, 2018


None yet
2 participants
Copy link

benjyw commented Dec 4, 2018

Note that the now-removed comments were already unnecessary, since the previously implicit use of the PythonSetup and PythonRepos subsystems is now explicit everywhere.

@benjyw benjyw requested review from cosmicexplorer and baroquebobcat Dec 4, 2018


stuhood approved these changes Dec 4, 2018

# We generally have to use PythonSetup's global instance, as methods such as
# `dump_requirements` will use it directly.
# TODO: when subsystems are more easily requestable from the v2 engine, this restriction could
# be removed!

This comment has been minimized.


stuhood Dec 4, 2018


Does removing this comment mean that this instance is no longer required either? Or is it still used in other spots?

This comment has been minimized.


benjyw Dec 5, 2018


The instance is still used, that comment was introduced in 6b11c59 to explain a switch from a scoped instance to the global instance. It seemed weird to me to have it in the code (as opposed to in the commit message for that change) because the casual reader doesn't know the history of the file and so this is addressing something they aren't wondering about to begin with. We could leave it in there as a warning against switching back to using the scoped instance, but that seems rather ad-hoc. We'd have to add that warning all over the place...

@benjyw benjyw merged commit f538b34 into pantsbuild:master Dec 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment