Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
bpo-40328: Add tool for generating cjk mapping headers#19602
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
corona10 commentedApr 19, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@vstinner |
vstinner left a comment
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.
I would prefer to put this script in Tools/unicode/ and don't store Unicode files in Modules/cjkcodecs/tools/data/CP932.TXT, but always download them: as done by Tools/unicode/makeunicodedata.py or some unit tests.
corona10 left a comment
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.
@vstinner Please take a look :)
corona10 commentedApr 21, 2020
@methane@serhiy-storchaka |
methane commentedApr 23, 2020
I don't have time to understand the script. I only confirmed that this pull request doesn't change the mapping header files. |
corona10 commentedApr 23, 2020
@methane |
corona10 commentedApr 29, 2020
@vstinner Can we merge this PR? |
vstinner left a comment
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.
Should we run this script in "make regen-all"? Or do you expect that these generated files will never ever change?
Currently, "make regen-all" has no 3rd party dependencies, whereas this script requires to download files from the Internet. In my experience, any additional external dependency makes Python CIs less stable :-(
Other "unicode scripts" are no run by "make regen-all", so I think that it's fine.
vstinner commentedApr 29, 2020
@corona10: I let you decide if you are confident enough to merge this PR with no review. Since this script is not run by any CI and only run manually, I don't see any risk of regression. It seems like "Unicode experts" (including me) don't have the bandwidth to review your change. |
Uh oh!
There was an error while loading.Please reload this page.
https://bugs.python.org/issue40328