RFR: 8184693: (opt) add Optional.isEmpty

Roger Riggs roger.riggs at oracle.com
Wed Apr 18 19:27:21 UTC 2018


+1

There is one extra space in Optional.java: line 163:  "is__not".

Thanks, Roger


On 4/18/18 2:06 AM, Stuart Marks wrote:
> 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
>>>>



More information about the core-libs-dev mailing list