gh-133503: Update compileall.rst's documentation of -s and -p for clarity,
#134756
Conversation
…-p` for clarity
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
@picnixz IMO it should be a warning considering it could cause breaking changes to programs if people don't pay close attention |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
I don't see how it's a breaking change. This has been the default behavior for ages. And warnings boxes should be used carefully as they disrupt the reading flow. |
|
Ok! I am doing it. Please review in a minute |
|
I'll review tomorrow. It's 2 AM for me |
|
Sorry! It's only 19:52 for me *I love 24 hr time even though I live in the US* |
|
@picnixz Get some sleep and please review in your morning :) |
There was a problem hiding this comment.
I think the documentation is inconsistent now. Previously -s and -p were documented as a single paragraph following both options. Now -s is followed by a comment, then -p is followed by the description of both options. They should either be described separately, or both notes need to be placed below -p.
There was a problem hiding this comment.
I would suggest the following:
.. option:: -s strip_prefix
Remove the given prefix from paths recorded in the ``.pyc`` files.
Paths are made relative to the prefix.
This option can be used with ``-p`` but not with ``-d``.
.. option:: -p prepend_prefix
Append the given prefix to paths recorded in the ``.pyc`` files.
Use ``-p /`` to make the paths absolute.
This option can be used with ``-s`` but not with ``-d``.|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
@picnixz Fixed it |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
|
Thanks @sharktide for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @sharktide for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
(cherry picked from commit fe6f8a3) Co-authored-by: Rihaan Meher <meherrihaan@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-134996 is a backport of this pull request to the 3.13 branch. |
(cherry picked from commit fe6f8a3) Co-authored-by: Rihaan Meher <meherrihaan@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-134997 is a backport of this pull request to the 3.14 branch. |
This will close #133503
This PR simple updates
compileall.rstto alert users to these following risks as stated by @mgornyRemoving the prefix makes paths relative to it.
-s and -p can be used simultaneously.
-p / can be used to make the path absolute.
-ssurprisingly makes path relative #133503📚 Documentation preview 📚: https://cpython-previews--134756.org.readthedocs.build/