RFR: 8246592: Simplify checking of boolean file attributes

[Roger Riggs]

Looks good.

Thanks for the extra attention.

Roger


On 6/5/20 6:09 AM, Claes Redestad wrote:
> Hi Roger,
>
> yes, things were a bit inconsistent whether this is checking all or one.
> I went with all (even though all usage only asks for one) and updated
> to use plural consistently.
>
> /Claes
>
> On 2020-06-05 11:23, Roger Riggs wrote:
>> Hi Claes,
>>
>> Also, since the requested attribute is a bit mask the test should be
>> that all of the attributes are present, not just some, iIt might 
>> prevent a mistake down the line.
>> Probably the method name and argument names should be plural.
>>
>> Line: 120: should be
>>
>> 119 public boolean hasBooleanAttributes(File f, int attributes) { 120 
>> return (getBooleanAttributes(f) & attributes) == attributes; 121 }
>>
>> Thanks, Roger
>>
>>
>> On 6/4/20 2:24 PM, Claes Redestad wrote:
>>> Hi Roger,
>>>
>>> On 2020-06-04 18:05, Roger Riggs wrote:
>>>> Hi Claes,
>>>>
>>>> The code looks ok but I would rename the new method to 
>>>> 'hasBooleanAttribute'
>>>> since it is not returning the attribute but testing for it.
>>>
>>> good suggestion:
>>>
>>> http://cr.openjdk.java.net/~redestad/8246592/open.01/
>>>
>>>>
>>>> I think you can leave the 'public' modifiers to a separate issue.
>>>
>>> Ok.
>>>
>>> /Claes
>>>
>>>>
>>>> Regards, Roger
>>>>
>>>>
>>>>
>>>>
>>>> On 6/4/20 9:41 AM, Claes Redestad wrote:
>>>>>
>>>>>
>>>>> On 2020-06-04 15:26, Alan Bateman wrote:
>>>>>> On 04/06/2020 14:22, Roger Riggs wrote:
>>>>>>> Hi Claes,
>>>>>>>
>>>>>>> Not a review...
>>>>>>>
>>>>>>> You'll need a CSR for the API change.
>>>>>>>
>>>>>>> FileSystem.java: You'll need proper javadoc for the new 
>>>>>>> getBooleanAttribute method.
>>>>>> FileSystem is not a public class so it's not actually an API 
>>>>>> change. I agree the public modifier is confusing, should probably 
>>>>>> be dropped from all these methods.
>>>>>
>>>>> If you prefer I can strip out all the public modifiers. Or do that 
>>>>> as a
>>>>> follow-up.
>>>>>
>>>>> /Claes
>>>>
>>