RFR: 8184693: (opt) add Optional.isEmpty

[Stuart Marks]

OK, looks good! +1 from me.

s'marks

On 4/17/18 10:34 PM, Vivek Theeyarath wrote:
> Hi Stuart,
> 	Done with the changes http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ .
> Regards
> Vivek
> -----Original Message-----
> From: Stuart Marks
> Sent: Wednesday, April 18, 2018 8:56 AM
> To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Paul Sandoz <paul.sandoz at oracle.com>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> Hi Vivek,
> 
> Thanks for the update. In the test files, please remove the unnecessary imports of List and the various Predicate types. In most cases it's not a problem to have unnecessary imports. I happened to notice in this case that they're left over from the previous version of the webrev, and looking at the current webrev by itself, it's clear they're unnecessary.
> 
> Thanks,
> 
> s'marks
> 
> On 4/17/18 6:23 AM, Vivek Theeyarath wrote:
>> Hi All,
>> 	Please find the updated webrev
>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04
>> Regards
>> Vivek
>>
>> -----Original Message-----
>> From: Stuart Marks
>> Sent: Tuesday, April 17, 2018 5:11 AM
>> To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
>> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Paul Sandoz
>> <paul.sandoz at oracle.com>
>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>
>> Hi Vivek,
>>
>> Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() methods.
>>
>> Regarding the tests, I don't think the various newly added testIsEmpty() tests are useful. The setup in those test files already generates a fairly comprehensive set of Optional values from a variety of sources. All the assertion checking is performed in the checkEmpty() and checkPresent() methods.
>> It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- as you've done -- and also to add an assertFalse(isEmpty()) call to checkPresent(). If these assertions are added, the additional testIsEmpty() test is unnecessary.
>>
>> This applies to all four of the regression test files.
>>
>> Thanks,
>>
>> s'marks
>>
>> On 4/16/18 11:08 AM, Vivek Theeyarath wrote:
>>> Hi All,
>>> 	Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
>>> Here is the csr which I have raised for this change
>>> https://bugs.openjdk.java.net/browse/JDK-8201606
>>>
>>> Regards
>>> Vivek
>>> -----Original Message-----
>>> From: Chris Hegarty
>>> Sent: Sunday, April 15, 2018 6:48 PM
>>> To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
>>> Cc: Remi Forax <forax at univ-mlv.fr>; core-libs-dev
>>> <core-libs-dev at openjdk.java.net>
>>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>>
>>>
>>>
>>>> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyarath at oracle.com> wrote:
>>>>
>>>> Hi All,
>>>>       Please review
>>>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/
>>>
>>> This looks ok to me.
>>>
>>> For consistency, can you please update the copyright header year range in OptionalInt.
>>>
>>> -Chris.
>>>
>>>> Regards
>>>> Vivek
>>>> -----Original Message-----
>>>> From: Vivek Theeyarath
>>>> Sent: Saturday, April 14, 2018 6:24 PM
>>>> To: Remi Forax <forax at univ-mlv.fr>
>>>> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
>>>> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
>>>>
>>>> I missed that Remi. Thanks for pointing it out. Will address those and get back.
>>>>
>>>> Regards
>>>> Vivek
>>>> -----Original Message-----
>>>> From: Remi Forax [mailto:forax at univ-mlv.fr]
>>>> Sent: Saturday, April 14, 2018 2:58 PM
>>>> To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
>>>> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
>>>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>>>
>>>> Hi Vivek,
>>>> OptionalInt, OptionalLong and OptionalDouble should be changed too.
>>>>
>>>> Rémi
>>>>
>>>> ----- Mail original -----
>>>>> De: "Vivek Theeyarath" <vivek.theeyarath at oracle.com>
>>>>> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>>>>> Envoyé: Samedi 14 Avril 2018 08:22:50
>>>>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
>>>>
>>>>> Hi All,
>>>>>
>>>>>                 Please review.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>>>>>
>>>>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>>>>>
>>>>>
>>>>>
>>>>> The related jtreg test was run and the test passed .
>>>>>
>>>>>
>>>>>
>>>>> Regards
>>>>>
>>>>> Vivek
>>>