Deprecate --build-dir by sbidoul · Pull Request #8372 · pypa/pip (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits1 Checks0 Files changed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
| 'The location of temporary directories can be controlled by setting ' |
|---|
| 'the TMPDIR environment variable (TEMP on Windows) appropriately. ' |
| 'When passed, build directories are not cleaned in case of failures.' |
| help=SUPPRESS_HELP, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be better to add (deprecated) to the description, instead of suppressing it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reasoning was to hide a deprecated option from new users and simplify the documentation, under the assumption that those who use it know what it is about.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern with that if someone is researching how the replacement would work, supressing this option's help makes this information (about the current behavior of the flag) inaccessible for all users, not just new users.
I feel "you're using it therefore you know the nuances of how it works and how to replace it" is a very... not-friendly approach toward any user who's affected by this deprecation.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done. I don't have a strong opinion on this. I personally find the help text for --build incomprehensible 🐱
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! ^>^
I personally find the help text for --build incomprehensible 🐱
You're not alone. We'll all sigh collectively when we remove this option entirely. :)
I have a feeling we might need more tools around script to check deprecation messages. Adding allow_stderr can be problematic since we’d have trouble remembering to remove the flag after the flag is fully removed 😐
| @@ -201,6 +201,7 @@ def test_no_clean_option_blocks_cleaning_after_wheel(script, data): |
|---|
| '--find-links={data.find_links}'.format(**locals()), |
| 'simple', |
| expect_temp=True, |
| allow_stderr_warning=True, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment saying "remove allow_stderr_warning, which is used for the --build deprecation"?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, in this case I think a comment is enough because this test will have to be revisited anyway when --build is removed.
FAILED tests/functional/test_install_cleanup.py::test_no_clean_option_blocks_cleaning_after_install
@sbidoul This also needs the same allowance.
bors bot referenced this pull request in duckinator/emanate
bors bot referenced this pull request in duckinator/emanate
This was referenced
Dec 14, 2020
