On Tue, 9 Feb 2021 08:28:57 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> Dear Serguei,
>> 
>>> There are two checks for cntTokens > 2. The second check has to be removed.
>>> Also, a return needs to be added after the line 1779 with "usage();" , so 
>>> the "else" statement can be removed.
>>> This has to be refactored to something like this:
>> 
>> Nice catch and good suggestion. Thanks a lot for point it out!
>> I think It is no hurry to push this PR, and welcome for any comment.
>> Thanks!
>
> Hi Lin,
> Thank you for update.
> 
> test/lib/jdk/test/lib/hprof/parser/Reader.java:
> +            } else if ((i >>> 8) == 0x1f8b08) {
> +                // Possible gziped file.
> 
> Could you, please, define a named constant and use it instead of the literal 
> value 0x1f8b08?
> Also, would it make sense to extend the comment to explain about it a little 
> bit?
> 
> I'm still looking at some files (e.g., HeapHprofBinWriter.java).
> 
> Thanks,
> Serguei

Dear @sspitsyn, 
Thanks for your suggestion. 

> I'm still looking at some files (e.g., HeapHprofBinWriter.java).

No problem, please take your time. Appreciated for your effort helping review! 

BRs,
Lin

-------------

PR: https://git.openjdk.java.net/jdk/pull/1712

Reply via email to