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
