Joe / Chris: The updated webrev is in the same place, http://cr.openjdk.java.net/~bpb/8020539/
If this looks good I will need an approval. On Jul 29, 2013, at 12:05 AM, Joe Darcy wrote: > I recommend a few changes here: > > * For readability, rename "NonASCIIDigit" as "NonAsciiDigit" Changed. > * Since Scanner is in the regex package, it may be more appropriate to stick > more of a regex notation than an ebnf notation, but keeping the definition > list structure tags. Changed back to regex notation. >> >>> Offhand, I wasn't aware Dequeue and Queue were maintained in Doug's CVS, >>> but I agree changes to those files should flow through there first. >> Deque and Queue removed from the webrev: >> >> http://cr.openjdk.java.net/~bpb/8020539/ >> > > Otherwise, the changes look fine. If you like, the non-Scanner changes can be > split out and pushed first. I've left it as one changeset. On Jul 29, 2013, at 2:43 AM, Chris Hegarty wrote: > Looks fine ( I did not go through the Scanner changes ). > > Some minor comments: > > 1) List > Trivially I wonder why you decided to go with '< '>', rather > than <pre>{@code ... }</pre>, which I think is more readable. Changed. > 2) Map > Trivially, I would keep the @param's order the same as the actual > method parameters order. Changed. > 3) Optional > Strangely, there doesn't appear to be a '@param' for the class level > type T. doclint is not warning about this either. Not changed. Thanks, Brian
