Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

2024-02-23 Thread Kevin Rushforth
On Fri, 23 Feb 2024 22:32:23 GMT, Andy Goryachev  wrote:

>> `@return` statements usually state this condition explicitly, like 
>> `Map#get`: "the value to which the specified key is mapped, or `null` if 
>> this map contains no mapping for the key".
>
> My point is that it should be mentioned in the first paragraph then.  It can 
> be repeated in @return later, if you prefer.  Map.get() does indeed mention 
> this in the first paragraph:
> 
> ![Screenshot 2024-02-23 at 14 30 
> 02](https://github.com/openjdk/jfx/assets/107069028/977a6dbc-c0b0-4a00-a14d-8ff40353c365)

Given that the changes are so far limited to spelling and formatting (code 
style for null), I think it would be fine to limit the changes in this PR to 
what he has already done.

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1379#discussion_r1501267342


Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

2024-02-23 Thread Andy Goryachev
On Fri, 23 Feb 2024 22:32:16 GMT, Nir Lisker  wrote:

>> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs 
>> in `Node` and `Scene`.
>> 
>> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
>> system property, while for `Node` it isn't (which means `SubScene` will be 
>> different from `Scene`). Not sure if this is intended.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments

Marked as reviewed by angorya (Reviewer).

-

PR Review: https://git.openjdk.org/jfx/pull/1379#pullrequestreview-1899094754


Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

2024-02-23 Thread Kevin Rushforth
On Fri, 23 Feb 2024 22:32:16 GMT, Nir Lisker  wrote:

>> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs 
>> in `Node` and `Scene`.
>> 
>> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
>> system property, while for `Node` it isn't (which means `SubScene` will be 
>> different from `Scene`). Not sure if this is intended.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments

Marked as reviewed by kcr (Lead).

-

PR Review: https://git.openjdk.org/jfx/pull/1379#pullrequestreview-1899088659


Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

2024-02-23 Thread Andy Goryachev
On Fri, 23 Feb 2024 21:58:30 GMT, Nir Lisker  wrote:

>> I don't mind either way.
>
> `@return` statements usually state this condition explicitly, like `Map#get`: 
> "the value to which the specified key is mapped, or `null` if this map 
> contains no mapping for the key".

My point is that it should be mentioned in the first paragraph then.  It can be 
repeated in @return later, if you prefer.  Map.get() does indeed mention this 
in the first paragraph:

![Screenshot 2024-02-23 at 14 30 
02](https://github.com/openjdk/jfx/assets/107069028/977a6dbc-c0b0-4a00-a14d-8ff40353c365)

-

PR Review Comment: https://git.openjdk.org/jfx/pull/1379#discussion_r1501237620


Re: RFR: 8325550: Grammatical error in AnchorPane.setLeftAnchor (and other setters) javadoc [v2]

2024-02-23 Thread Nir Lisker
> Fixes for the `AnchorPane` docs, as well as for the `NodeOrientation` docs in 
> `Node` and `Scene`.
> 
> Note that the default value for a `Scene`'s `NodeOrientation` depends on a 
> system property, while for `Node` it isn't (which means `SubScene` will be 
> different from `Scene`). Not sure if this is intended.

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/1379/files
  - new: https://git.openjdk.org/jfx/pull/1379/files/dac77a49..a3c375df

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1379=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1379=00-01

  Stats: 53 lines in 2 files changed: 21 ins; 25 del; 7 mod
  Patch: https://git.openjdk.org/jfx/pull/1379.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1379/head:pull/1379

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