-
Notifications
You must be signed in to change notification settings - Fork 34
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
Proposed unsafe signatures for ICU #90
Comments
Cool! I let @rmuir look into it :-) |
There is only one issue: For commons-io we have a per-version file. Not sure if we need the same for ICU. |
Yeah... I did this for 55 but even we're going to roll back to 54 at some point so at that point I will have to split it into two files. I'm not sure about breaking it up all the way back to 1 though. ;) |
Maybe its time to have some @SInCE annotation in the files... |
the list looks good, yeah basically grep for Locale/TimeZone/Charset/etc and then reviewing classes using them is the way to find these. @uschindler I think the version file would be good, the reason is new calendars get added and so on. |
You almost want a 'since' and 'until' annotation because some libraries do eventually remove API (even though the JRE is adverse to removing even the most cancerous growths...) |
Oh with @deprecated we have seen removal already. For the @deprecated ones there is a @Suppress annotation used internally to suppress warnings appearing because of removal :-) |
I just don't want to have a file for every 55 major version... I will think about a better solution for bundled signatures. Also commons-io is a mess currently. Every newer version is including the previous ones! For 55 this is also leading to slowdown caused by the chain of includes. |
I still have no idea about how to fix versioning. I will open separate issue for that and during that issue "merge" all version specific signatures files into one. About the additional calendars: Maybe this is another reason for #88 (glob patterns also allowed in combination with methods). You could then add some signature like |
ICU mirrors a lot of the Java APIs and our own project has been using it a lot. (Initially, it was just because java.text.MessageFormat is inadequate for internationalising plurals in formats, but other usages have crept in over time. e.g., we use it to get at some additional calendars as well.)
I went through a javap dump of every method signature in ICU 55.1 and came up with the following list. Since commons-io is present out of the box, maybe ICU could be as well. :)
I don't know if there is a better way to find potential methods but what I did was search the list for methods taking TimeZone or Locale and then looked at any overloads or similar-looking methods in the same class. So maybe I have missed some... but at least it's smaller than the JRE, so it's comparatively easier to do this sort of exercise.
The text was updated successfully, but these errors were encountered: