Skip to content
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

deprecate --export-py-hermetic-scripts in favor of new --export-py-non-hermetic-scripts-in-resolve option #21181

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Jul 18, 2024

Deprecate the --export-py-hermetic-scripts option in favor of the new --export-py-non-hermetic-scripts-in-resolve option so that the hermetic script logic can be configured on a per-resolve basis. The behavior for the new option is inverted in a sense from the old option since if we named the option --export-py-hermetic-scripts-in-resolve then the default option value of an empty list would mean that resolve did not use hermetic scripts by default. Thus, the new option acts instead as an opt-in to use non-hermetic scripts.

@tdyas tdyas requested review from benjyw and cognifloyd July 18, 2024 21:32
@tdyas tdyas marked this pull request as ready for review July 18, 2024 21:32
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a minor question, but otherwise LGTM.

src/python/pants/backend/python/goals/export.py Outdated Show resolved Hide resolved
@cognifloyd
Copy link
Member

Should this be category: user api change instead of category: new feature?

@tdyas
Copy link
Contributor Author

tdyas commented Jul 18, 2024

Should this be category: user api change instead of category: new feature?

Ah didn't know that category existed when I selected. Will switch it.

@tdyas tdyas changed the title rename --export-py-hermetic-scripts to --export-py-non-hermetic-scripts-in-resolve deprecate --export-py-hermetic-scripts in favor of new --export-py-non-hermetic-scripts-in-resolve option Jul 19, 2024
@tdyas tdyas merged commit f151aaa into pantsbuild:main Jul 19, 2024
25 checks passed
@tdyas tdyas deleted the rename_export_py_hermetic_scripts branch July 19, 2024 03:06
huonw added a commit that referenced this pull request Sep 16, 2024
This `py_hermetic_scripts` option was deprecated in #21181 in favour of
the new `py_non_hermetic_scripts_in_resolve` option. This was merged for
2.23.0.dev6 and thus will come in 2.23.0 soon-ish...

It seems to be very little code to support `py_hermetic_scripts` (just a
single extra clause in an `if` statement, plus the option definition
itself), so it seems unnecessarily aggressive to deprecate in one cycle.
This extends it a few releases to make it easier for users to test
pre-releases.

For instance, someone using 2.22.0 (current stable) with
`py_hermetic_scripts` can currently test 2.23.0a0 with just a warning
(e.g. adjust `pants_version` and run it through their CI), but would be
blocked from doing _any_ testing of 2.24.0.dev0 (#21412) until they fix
this option. That is, by removing the option quickly, it's harder for
them to validate any part of the 2.24 or 2.25 release series and provide
us feedback while on older versions.

(We're being more permissive than what we were previosuly communicating,
so I don't think we need release notes or to otherwise highlight it.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants