On Sat, 30 Oct 2021 14:18:42 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

> there is no need for the class rename, is there?
> 
> Even though it's formally internal api, I think we shouldn't do code breaking 
> changes except if there's a very compelling reason - there are too many apps 
> out in the wild that use internal api.

Well, internal interfaces aren't API. With JDK 9 - 16 it requires relaxing 
encapsulation to get at it. As of JDK 17, it requires an explicit export of the 
package to access it. Having said that, since this is a fix we are likely to 
want to backport to at least JavaFX 17.0.X, I think we should minimize the 
scope of the fix.

So I agree with @kleopatra (albeit for a different reason) that we shouldn't 
make this change.

> I don't know how `TextBinding` describes in any way what this class is doing. 
> Naming is important, and I think `MnemonicParser`, on the other hand, 
> describes exactly what is going on.

Agreed.

> I would disagree that treating internal APIs as if they were public APIs is a 
> good place to be.

Completely agree.

> Cleaning up and refactoring internal APIs is important for the long-term 
> health of a codebase.

Yes, but I would prefer to see it as a separate fix, rather than as part of a 
(critical) regression bug fix.

-------------

PR: https://git.openjdk.java.net/jfx/pull/647

Reply via email to