RFR: 8229472: Deprecate for removal JavaBeanXxxPropertyBuilders constructors

[Kevin Rushforth]

On Tue, 19 Nov 2019 18:02:23 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> On Mon, 18 Nov 2019 18:12:13 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
> 
>> On Wed, 13 Nov 2019 23:47:58 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> 
>>> On Wed, 6 Nov 2019 12:54:40 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>> 
>>>> On Wed, 6 Nov 2019 11:50:27 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>>>> 
>>>>> On Wed, 6 Nov 2019 07:12:26 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:
>>>>> 
>>>>>> On Wed, 6 Nov 2019 05:07:56 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>>>>> 
>>>>>>> On Wed, 6 Nov 2019 05:04:28 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>>>>>> 
>>>>>>>> On Tue, 5 Nov 2019 18:10:57 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>>>>>>>> 
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8229472
>>>>>>>>> 
>>>>>>>>> CSR will be created after the changes are approved.
>>>>>>>>> 
>>>>>>>>> ----------------
>>>>>>>>> 
>>>>>>>>> Commits:
>>>>>>>>>  - 6d29e034: Initial push of 8229472
>>>>>>>>> 
>>>>>>>>> Changes: https://git.openjdk.java.net/jfx/pull/30/files
>>>>>>>>>  Webrev: https://webrevs.openjdk.java.net/jfx/30/webrev.00
>>>>>>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8229472
>>>>>>>>>   Stats: 14 lines in 7 files changed: 7 ins; 0 del; 7 mod
>>>>>>>>>   Patch: https://git.openjdk.java.net/jfx/pull/30.diff
>>>>>>>>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/30/head:pull/30
>>>>>>>> 
>>>>>>>> modules/javafx.base/src/main/java/javafx/beans/property/adapter/JavaBeanBooleanPropertyBuilder.java line 68:
>>>>>>>> 
>>>>>>>>> 67:     @Deprecated(since = "14", forRemoval = true)
>>>>>>>>> 68:     public JavaBeanBooleanPropertyBuilder() {}
>>>>>>>>> 69: 
>>>>>>>> 
>>>>>>>> Minor: I checked other places in JavaFX and JDK, and they consistently omit the spaces surrounding the `=`.
>>>>>>> 
>>>>>>> I changed the bug summary to include `for removal` in the title. Can you change the PR title to match?
>>>>>> 
>>>>>> Thanks for the notification, looks like GitHub returned 500 for a few minutes. This seem to happen from time to time, so nice to know that the retry logic works. :)
>>>>> 
>>>>> For both `since` and `forRemoval`?
>>>> 
>>>> Yes. The pattern used consistently (in all cases that I could find) would be:
>>>> 
>>>>     @Deprecated(since="14", forRemoval=true)
>>> 
>>> @arapte can you also review this?
>> 
>>> Please add @deprecated javadoc tag in the description for all constructors.
>> 
>> Good idea.
> 
> Good suggestion. I will change to
> 
> /**
> * @deprecated This constructor was exposed erroneously and will be removed in the next version. Use {@link #create()} instead.
> */
> @Deprecated(since="14", forRemoval=true)
> 
> I noticed that the `ReadOnlyJavaBeanXxxPropertyBuilder`s also have the same issue. Can I fix them in this PR too or do we need a new one?
> 
> Also, the CSR is finalized, can I change it at this point?

> I noticed that the ReadOnlyJavaBeanXxxPropertyBuilders also have the same issue. Can I fix them in this PR too or do we need a new one?

Good catch. Not sure how we missed this before now (the change for JavaFX 13 also missed it), but yes, I'd prefer to fix this now for the ReadOnlyJavaBeanXxxPropertyBuilders, too.

For the CSR, you can move it back to Draft, edit it, and then once the review is done, Finalize it again (no need to go through the Proposed phase a second time).

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