-
-
Notifications
You must be signed in to change notification settings - Fork 44
move UnicodeProperty so it doesn't shadow CLDRs #214
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
I think the very best approach would be to remove UnicodeProperty from CLDR
if possible. There are however lots of direct and indirect dependencies.
Second best would be to merge anything useful from the unicodetools one
into the CLDR one, and delete the unicodetools one.
|
Mark:
Yes, all as discussed in the top description of issue #66. I agree with moving the unicodetools version to a different package, to avoid the shadowing, but I strongly suspect that the version there needs to be mostly a copy of the CLDR version, in order to preserve behavior. Steven:
My comment from last year says that the CLDR version has a different signature. Either the Unicode Tools rely on that now, or they don't call anything where the API has changed. From #66:
|
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.
Looks good to me. A couple of questions.
Were the moved files like BagFormatter just copied, or were there more substantive changes?
Why was "final" removed from a number of declarations in unicodetools/src/main/java/org/unicode/jsp/UnicodeRegex.java? Was that because of merging with another version?
The ones from CLDR that are completely new were not changed. I can enumerate.
Merging between the unicodetools and the jsp version. |
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.
lgtm tnx!
For: #66
while working on #186 I found some tools wouldn't even work for me due to the shadowed classes issue discussed in #66
This untangles things a bit… while making other things a little more tangled.
Draft because I'm not sure about all of this, but it does pass tests, and GenerateIdna actually runs again instead of crashing.