Re: [PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Hello Stuart, On 19/09/18 11:06 PM, Stuart Marks wrote: >   > >> While adding this, I realized that the current javadoc doesn't mention >> anything about the behaviour of this method when a null array is passed >> to it. The implementation currently throws a NullPointerException. So I >> went

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On 9/19/18, 1:52 PM, Brian Burkhalter wrote: On Sep 19, 2018, at 1:44 PM, Brian Burkhalter wrote: On Sep 19, 2018, at 1:14 PM, Alan Bateman wrote: Starting out using buffered I/O is probably okay but I assume we will want to change this in the future to having it use memory mapped I/O

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On 9/19/18, 2:02 PM, Roger Riggs wrote: Hi Alan, On 9/19/18 4:14 PM, Alan Bateman wrote: On 19/09/2018 19:48, Joe Wang wrote: Hi, After much discussion and 10 iterations of reviews, this proposal has evolved from what was the original isSameContent method to a mismatch method. API-wise,

Re: RFR(XS): 8210925 : move bytecode testlibrary to top-level testlibrary

Hi Mandy, yes, eventually I'd like to see this library being used in place of ASM in our tests. I ain't planning to do it in JDK 12 TF though. if there is a newer version ready, I'm happy to update (and test) the library, although it shouldn't block testlibraries "merge" activity. -- Igor >

Re: [PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Stuart, OK, I didn't know you already had that in mind. -- Jon On 09/19/2018 03:23 PM, Stuart Marks wrote: Hi Jon, Yes, definitely, given the scope of the current proposed changes, I intend to file a CSR for this changeset on Jaikiran's behalf. s'marks On 9/19/18 10:42 AM, Jonathan

Re: RFR(XS): 8210925 : move bytecode testlibrary to top-level testlibrary

Hi Igor, Are you planning to convert the hotspot tests to use this bytecode library? This bytecode library is one version that was snapshot when these condy tests were integrated in JDK.   It has likely been evolved.   Maurizio and Robert may comment on this whether this bytecode library is

Re: [PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Hi Jon, Yes, definitely, given the scope of the current proposed changes, I intend to file a CSR for this changeset on Jaikiran's behalf. s'marks On 9/19/18 10:42 AM, Jonathan Gibbons wrote: Jaikiran, Referring back to the original email discussion, changes like this may require a CSR to

Re: RFR - JDK-8202442 - String::unescape (Code Review)

On 9/18/18 10:51 AM, Jim Laskey wrote: Please review the code for String::unescape. Used to translate escape sequences in a string, typically in a raw string literal, into characters represented by those escapes. webrev: http://cr.openjdk.java.net/~jlaskey/8202442/webrev/index.html jbs:

RFR(XS): 8210925 : move bytecode testlibrary to top-level testlibrary

http://cr.openjdk.java.net/~iignatyev//8210925/webrev.00/index.html > 14 lines changed: 0 ins; 0 del; 14 mod; Hi all, could you please review this small and trivial patch which moves bytecode testlibrary to top-level testlibrary directory? webrev:

RFR: JDK-8210924 Remove PACKAGE_PATH

The variable PACKAGE_PATH crept into the build when the macosx port was integrated. It is not used, however, and should be removed. (This is a BSD construct, and the macosx port was based on the BSD port.) If/when the BSD port ever gets integrated upstream, we'll know better then how to

Re: RFR(M) : 8210894 : remove jdk/testlibrary/Asserts

Looks fine. On 19/09/2018 10:18, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8210894/webrev.00/index.html 1188 lines changed: 252 ins; 591 del; 345 mod; Hi all, could you please review the next clean up of jdk testlibrary? now we are replacing all usages of

Re: RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

Thanks for looking at this. The proposed fix does not tackle the bug fully. The bug is that the spec says "The format consists of: The ISO_OFFSET_DATE_TIME where ..." As such, the format must parse *any* offset, not just "Z" / "+00:00" etc. In addition, the fix doesn't work properly. Parsers

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Hi Alan, On 9/19/18 4:14 PM, Alan Bateman wrote: On 19/09/2018 19:48, Joe Wang wrote: Hi, After much discussion and 10 iterations of reviews, this proposal has evolved from what was the original isSameContent method to a mismatch method. API-wise, a compare method was also considered as it

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On Sep 19, 2018, at 1:52 PM, Brian Burkhalter wrote: >> For the buffered case, could there be any performance advantage to using >> direct buffers with FileChannel and comparing running checksums of the two >> sources? > > Don’t think this would work however as two files with differing

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On Sep 19, 2018, at 1:44 PM, Brian Burkhalter wrote: > On Sep 19, 2018, at 1:14 PM, Alan Bateman wrote: > >> Starting out using buffered I/O is probably okay but I assume we will want >> to change this in the future to having it use memory mapped I/O beyond a >> certain threshold. > > For

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On Sep 19, 2018, at 1:14 PM, Alan Bateman wrote: > Starting out using buffered I/O is probably okay but I assume we will want to > change this in the future to having it use memory mapped I/O beyond a certain > threshold. For the buffered case, could there be any performance advantage to

Re: RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On 19/09/2018 19:48, Joe Wang wrote: Hi, After much discussion and 10 iterations of reviews, this proposal has evolved from what was the original isSameContent method to a mismatch method. API-wise, a compare method was also considered as it looked like just a short step forward from

Re: RFR - JDK-8210717 String::detab, String::entab (Code Review)

It should really be "if n is less than or equal to zero" (equal, not equals) -- Jon On 09/19/2018 05:35 AM, Jim Laskey wrote: Thank you. Updated. On Sep 19, 2018, at 4:20 AM, Andrej Golovnin wrote: Hi Jim, I'm not native english speaker. But that does not seem to be right for me: 2983

Re: RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

Hi Pallavi, Thank you for working on this. I think the javadoc should also be updated, mentioning offset parsing. Naoto On 9/19/18 10:15 AM, Pallavi Sonal wrote: Hi, Please review the changes to the following issue: Bug : https://bugs.openjdk.java.net/browse/JDK-8166138 The proposed

Re: RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

Hi Pallavi, On 9/19/18 1:15 PM, Pallavi Sonal wrote: Hi, Please review the changes to the following issue: Bug : https://bugs.openjdk.java.net/browse/JDK-8166138 The proposed fix is located at: Webrev : http://cr.openjdk.java.net/~rpatil/8166138/webrev.00/

RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Hi, After much discussion and 10 iterations of reviews, this proposal has evolved from what was the original isSameContent method to a mismatch method. API-wise, a compare method was also considered as it looked like just a short step forward from mismatch, however, it was eventually dropped

Re: [PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Jaikiran, Referring back to the original email discussion, changes like this may require a CSR to be filed. From Stuart Marks: http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-August/054894.html Whether this requires a CSR depends on whether any normative text of the specification

Re: [PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Hi Jaikiran, Thanks for doing the updates and sending the patch. Replies to your comments follow. While adding this, I realized that the current javadoc doesn't mention anything about the behaviour of this method when a null array is passed to it. The implementation currently throws a

RFR(M) : 8210894 : remove jdk/testlibrary/Asserts

http://cr.openjdk.java.net/~iignatyev//8210894/webrev.00/index.html > 1188 lines changed: 252 ins; 591 del; 345 mod; Hi all, could you please review the next clean up of jdk testlibrary? now we are replacing all usages of jdk.testlibrary.Asserts w/ jdk.test.lib.Assert class and remove

RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

Hi, Please review the changes to the following issue: Bug : https://bugs.openjdk.java.net/browse/JDK-8166138 The proposed fix is located at: Webrev : http://cr.openjdk.java.net/~rpatil/8166138/webrev.00/ As per ISO 8601 standards, an offset of zero, in addition to having the special

Re: [12] RFR: 8209880: tzdb.dat is not reproducibly built

Hi, On 09/18/2018 07:22 PM, Erik Joelsson wrote: Looks good to me. I guess it's hard to prove that the output is now stable, but time will tell. /Erik On 2018-09-18 10:41, Naoto Sato wrote: Hello, Please review the fix to the following issue:

Re: RFR - JDK-8203442 String::transform (Code Review)

updated webrev: http://cr.openjdk.java.net/~jlaskey/8203442/webrev-01/index.html > On Sep 19, 2018, at 10:35 AM, Jim Laskey wrote: > >> On Sep 19, 2018, at 9:58 AM, Remi Forax wrote: >> >> Hi Jim, >> the signature of

Re: RFR - JDK-8210717 String::detab, String::entab (Code Review)

webrev updated http://cr.openjdk.java.net/~jlaskey/8210717/webrev-01/index.html > On Sep 19, 2018, at 9:35 AM, Jim Laskey wrote: > > Thank you. Updated. > >> On Sep 19, 2018, at 4:20 AM, Andrej Golovnin >> wrote: >> >>

Re: RFR - JDK-8210718 String::detab, String::entab (CSR Review)

> On Sep 18, 2018, at 4:40 PM, Roger Riggs wrote: > > Hi Jim, > > It may be useful to be more specific about the definition of a tab stop. Agree. > In this context I think you mean the next multiple of 'n' greater than the > current position + 1. > (And not taking into account surrogates,

Re: RFR - JDK-8202442 - String::unescape (Code Review)

Will do. > On Sep 19, 2018, at 11:42 AM, Jonathan Gibbons > wrote: > > If you're making the change, then you might add a test case based on Jan's > suggestion, testing > > "\u005ct".equals(`\u005ct`.unescape()) > > -- Jon > > > On 9/19/18 7:40 AM, Jim Laskey wrote: >> I see and good

Re: RFR - JDK-8202442 - String::unescape (Code Review)

If you're making the change, then you might add a test case based on Jan's suggestion, testing     "\u005ct".equals(`\u005ct`.unescape()) -- Jon On 9/19/18 7:40 AM, Jim Laskey wrote: I see and good point. I think Jon is correct. If you are using the JLS as the basis of your argument,

Re: RFR - JDK-8202442 - String::unescape (Code Review)

I see and good point. I think Jon is correct. If you are using the JLS as the basis of your argument, then you better damn well stick to the JLS. I’ll make the changes. Cheers, — Jim > On Sep 19, 2018, at 11:10 AM, Jan Lahoda wrote: > > I guess Jon's comment was that (per JLS) the

Re: RFR JDK-8210899: (zipfs) ZipFileSystem.EntryOutputStreamCRC32 mistakenly set the crc32 value into size field

Hi Sherman On Sep 18, 2018, at 6:45 PM, Xueming Shen wrote: > Thanks looking into it. You’re welcome. > Yes, the current code (with the change for JDK-8034802) fails both of the > added > checks. Thanks for the update. Brian

Re: RFR - JDK-8202442 - String::unescape (Code Review)

Jan, Yes, thanks for clarifying my comment. And to repeat, my comment was just to raise awareness in the different treatment according to JLS and to .unescape(). -- Jon On 9/19/18 7:10 AM, Jan Lahoda wrote: I guess Jon's comment was that (per JLS) the outcome of unicode unescapes can then

Re: RFR - JDK-8202442 - String::unescape (Code Review)

I guess Jon's comment was that (per JLS) the outcome of unicode unescapes can then participate in the escape sequences in String literals. So, this: "\u005ct" is (as far as I know) a single character-literal (a tab), while it seems that `\u005ct`.unescape() is two characters: \t Not sure

Re: RFR - JDK-8203703 String::transform (CSR Review)

I’m not married to the name (I suggested apply). “with” also works. The originating goal was to allow custom alignment methods for those developers not satisfied with String::align(). String would logically be the result. “transform" became generic when the case was made that other types might

Re: RFR - JDK-8203442 String::transform (Code Review)

> On Sep 19, 2018, at 9:58 AM, Remi Forax wrote: > > Hi Jim, > the signature of transform() in the webrev was not updated (so the wildcards > are missing). Apologies. I created the webrev before I fully saved. Will update in a bit. > > And i'm still not convince this method should be

Re: RFR - JDK-8203703 String::transform (CSR Review)

On Tue, 18 Sep 2018 at 18:57, Jim Laskey wrote: > > Please review the API for String::transform. The goal is to provide a String > instance method to allow function application of custom transformations > applied to an instance of String. > > csr:

Re: RFR - JDK-8203442 String::transform (Code Review)

Hi Jim, the signature of transform() in the webrev was not updated (so the wildcards are missing). And i'm still not convince this method should be introduced as is: - it need more variants (transformToInt, transformToLong, transformToDouble) to be useful, currently

Re: RFR - JDK-8210717 String::detab, String::entab (Code Review)

Thank you. Updated. > On Sep 19, 2018, at 4:20 AM, Andrej Golovnin > wrote: > > Hi Jim, > > I'm not native english speaker. But that does not seem to be right for me: > > 2983 * @throws IllegalArgumentException if n is less that equals to zero. > > 3029 * @throws

Re: RFR - JDK-8202442 - String::unescape (Code Review)

Updated. > On Sep 18, 2018, at 5:55 PM, Naoto Sato wrote: > > Hi Jim, > > At the last sentence in the method description, there's a line "otherwise a > CharacterCodingException may occur during UTF-8 decoding." It's not decoding > into UTF-8, so simply "during decoding (or unescaping)"

Re: [12] RFR: 8209880: tzdb.dat is not reproducibly built

On 2018-09-18 19:41, Naoto Sato wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8209880 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8209880/webrev.00/ Looks good. Thank you for fixing this! /Magnus The

Re: RFR - JDK-8210717 String::detab, String::entab (Code Review)

Hi Jim, I'm not native english speaker. But that does not seem to be right for me: 2983 * @throws IllegalArgumentException if n is less that equals to zero. 3029 * @throws IllegalArgumentException if n is less that equals to zero. I would write it as: if n is less than or equals to