-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
bpo-40328: Add tool for generating cjk mapping headers #19602
Conversation
@vstinner |
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.
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 :)
@methane @serhiy-storchaka |
I don't have time to understand the script. I only confirmed that this pull request doesn't change the mapping header files. |
@methane |
@vstinner Can we merge this PR? |
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.
@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. |
https://bugs.python.org/issue40328