Re: [jfx19] RFR: 8286678: Fix mistakes in FX API docs [v4]

2022-08-23 Thread Kevin Rushforth
On Tue, 23 Aug 2022 15:36:35 GMT, Nir Lisker  wrote:

>> Fixes the mistakes in the JBS ticket and some additional minor corrections.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments

Looks good with one optional suggestion.

modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java 
line 50:

> 48:  * setting a value will result in an {@code IllegalStateException} and 
> the value is always {@code null}.
> 49:  * 
> 50:  * Since {@code Formatter} contains a value which represents the state of 
> the {@code TextInputControl} to which it is

One more thing I noticed: "contains a value which" could be changed to 
"contains a value that"

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.org/jfx/pull/880


Re: [jfx19] RFR: 8286678: Fix mistakes in FX API docs [v4]

2022-08-23 Thread Kevin Rushforth
On Tue, 23 Aug 2022 15:36:35 GMT, Nir Lisker  wrote:

>> Fixes the mistakes in the JBS ticket and some additional minor corrections.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments

> When looking at `Service` I noticed that in `Worker` the property docs are on 
> the `get` methods instead of on the `property` methods ...
> 
> This causes `Service` to inherit these as well. Docs should only appear on 
> the property (except in special circumstances), but I didn't want to touch 
> all of this here. Should I create a new issue for this or is it not worth it?

I recommend to file a new issue. We can target it for JavaFX 20.

-

PR: https://git.openjdk.org/jfx/pull/880


Re: [jfx19] RFR: 8286678: Fix mistakes in FX API docs [v4]

2022-08-23 Thread Nir Lisker
On Tue, 23 Aug 2022 15:36:35 GMT, Nir Lisker  wrote:

>> Fixes the mistakes in the JBS ticket and some additional minor corrections.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments

When looking at `Service` I noticed that in `Worker` the property docs are on 
the  `get` methods instead of on the `property` methods: 
https://openjfx.io/javadoc/18/javafx.graphics/javafx/concurrent/Worker.html#getState()

This causes `Service` to inherit these as well. Docs should only appear on the 
property (except in special circumstances), but I didn't want to touch all of 
this here. Should I create a new issue for this or is it not worth it?

-

PR: https://git.openjdk.org/jfx/pull/880


Re: [jfx19] RFR: 8286678: Fix mistakes in FX API docs [v4]

2022-08-23 Thread Nir Lisker
> Fixes the mistakes in the JBS ticket and some additional minor corrections.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressed review comments

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/880/files
  - new: https://git.openjdk.org/jfx/pull/880/files/6e9eda88..e9df3a27

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=880&range=03
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=880&range=02-03

  Stats: 20 lines in 1 file changed: 0 ins; 1 del; 19 mod
  Patch: https://git.openjdk.org/jfx/pull/880.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/880/head:pull/880

PR: https://git.openjdk.org/jfx/pull/880