On Sun, 20 Jun 2021 00:22:57 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/javafx/css/Stylesheet.java line 301:
>> 
>>> 299:      * css version or if an I/O error occurs while reading from the 
>>> stream
>>> 300:      */
>>> 301:     public static Stylesheet loadBinary(InputStream stream) throws 
>>> IOException {
>> 
>> Why do you need to add this public method to the API? I don't see any 
>> discussion as to why JavaFX applications need it. It looks like it is just 
>> being used internally by `StyleManager`. Unless there is a compelling reason 
>> to add this to the API, you will need to make this method package-scope and 
>> use an accessor to access it from `StyleManager`.
>
> I don't know why JavaFX applications would need this API. But the argument 
> should be the same argument as for `loadBinary(URL)`, which is also only used 
> internally by `StyleManager`. I think that this isn't opening up any new 
> effective API surface, because it's arguably just another overload of the 
> existing API.

An overloaded method with a new signature is still a new method that adds to 
the public API, although in an easy to understand way. I agree that the same 
argument could be made for it as for `loadBinary(URL)`, but if neither are 
needed, then maybe we would be better off to deprecate the existing method (not 
as part of this enhancement, of course), rather than adding a similar one that 
also isn't needed as part of the public API. On the other hand, if there is a 
good reason, I don't see a problem adding this new method.

If you do propose to add this API, you will need to add an appropriate `@since` 
tag, and you should write an API-level test for this method as part of 
`StylesheetTest`.

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

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

Reply via email to