-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-36700: Updated obsolete references for RFC 3548 to RFC 4648
#2336
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
@paulehoffman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @serhiy-storchaka and @Carreau to be potential reviewers. |
I signed the CLA earlier today, so I'm not sure why the bot didn't find it. |
Your indent on line 133 appears to have offended the CI bot for some reason. (just looked, it's an extra space at the start of the line. Probably want to fix that.) |
Lib/base64.py
Outdated
|
||
# Modified 04-Oct-1995 by Jack Jansen to use binascii module | ||
# Modified 30-Dec-2003 by Barry Warsaw to add full RFC 3548 support | ||
# Modified 30-Dec-2003 by Barry Warsaw to add full RFC 4648 support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...that appears to be an incorrect change, rewriting history...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I have reverted that in d585264.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue on the bug tracker for discussion.
We should check that the module supports the new RFC, not just update the number.
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 |
@paulehoffman Thank you for the suggested change and for the PR! If you've opened a bug tracker ticket per @serhiy-storchaka's review, please update the PR title with the bpo number and remove the |
I have made the requested changes; please review again |
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
(oh), and for optional mapping of the digit 1 (one) to either the letter I (eye) | ||
or letter L (el). The optional argument *map01* when not ``None``, specifies | ||
which letter the digit 1 should be mapped to (when *map01* is not ``None``, the | ||
digit 0 is always mapped to the letter O). For security purposes the default is | ||
``None``, so that 0 and 1 are not allowed in the input. | ||
``None``, so that 0 and 1 are not allowed in the input. Note that :rfc:`4648` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reas 8000 on for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is also not mentioned in the Changes since RFC 3548 section. I think we can leave the original wording on these pages and just update the number.
@paulehoffman, please address the code review and resolve the merge conflicts. Thanks! |
This PR is quite old, but the blocker is now fixed. @paulehoffman do you mind rebasing it against master and addressing this comment? |
Updating just the documentation for base64 library. The RFC that was reference was obsolete. The description of both the old and new RFCs was incorrect in saying that they allowed optional changes; the proposed new wording makes this clear without deprecating the option.
https://bugs.python.org/issue36700