On Wed, 12 Jul 2023 16:17:49 GMT, Glavo <d...@openjdk.org> wrote: >> Maybe a small suggestion to make it clear whats wanted here. In other >> projects I am involved in (Apache Lucene/Solr, Apache TIKA, PostgresSQL >> JDBC, Checkstyle itsself, Elasticserach/Opensearch), which use the >> [forbiddenapis Maven/Gradle/Ant >> plugin](https://github.com/policeman-tools/forbidden-apis/), we forbid all >> calls to several Java APIs (including toLowerCase/toUpperCase case). All >> bytecode using this will build failure (FYI, we also disallow other stuff >> like relying of default timezone or characterset). >> To make it clear what is really intended, those projects agreed on having >> `toLowerCase(Locale.getDefault())`, so it is explicit what's wanted. >> Without that it could be that somebody else starts the discussion again. >> >> This is just a suggestion to be explicit as it makes maintaining the code >> easier. > >> Maybe a small suggestion to make it clear whats wanted here. In other >> projects I am involved in (Apache Lucene/Solr, Apache TIKA, PostgresSQL >> JDBC, Checkstyle itsself, Elasticserach/Opensearch), which use the >> [forbiddenapis Maven/Gradle/Ant >> plugin](https://github.com/policeman-tools/forbidden-apis/), we forbid all >> calls to several Java APIs (including toLowerCase/toUpperCase case). All >> bytecode using this will build failure (FYI, we also disallow other stuff >> like relying of default timezone or characterset). To make it clear what is >> really intended, those projects agreed on having >> `toLowerCase(Locale.getDefault())`, so it is explicit what's wanted. Without >> that it could be that somebody else starts the discussion again. >> >> This is just a suggestion to be explicit as it makes maintaining the code >> easier. > > I agree with this. > > I'm working on deprecating `toLowerCase()` and `toUpperCase()`, this PR is > part of that effort. I wish to convert all use cases of them to > `toLowerCase(Locale)` and `toUpperCase(Locale)`. > > More backstory is detailed in > https://github.com/openjdk/jdk/pull/13434#issuecomment-1503989660.
> However, while I think this corrects the behavior, this caused a change in > the behavior of the API, so a CSR may be required. I don't want to debate > this in this PR, so I'll revert this change and open a new PR in the future. StreamTokenizer is a very old API and changing long standing behavior may break something or be observable with existing code/usages. I see youve reverted this part (thanks) and looking at it separately is fine. It might be that the conclusion is that it's just too risky to change, in which case Uwe's suggestion is good and would avoid it showing up on someone's else radar in the future. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14763#discussion_r1263570865
