[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8235530: Removed duplicated threadByName methods in nsk/jdi tests
From: coleen.phillimore () oracle ! com
Date: 2019-12-09 21:04:33
Message-ID: ca238477-d069-6601-45e5-9521e706b8e0 () oracle ! com
[Download RAW message or body]
Very nice!
4153 lines changed: 21 ins; 3841 del; 291 mod; 99816 unchg
Coleen
On 12/9/19 3:56 PM, Leonid Mesnik wrote:
> David, Serguei
>
> Thank you for review. I added comment about JDITestRuntimeException in
> https://bugs.openjdk.java.net/browse/JDK-8235544
>
> Leonid
>
> On 12/7/19 9:19 PM, David Holmes wrote:
>> +1 on both counts
>>
>> Not sure JDITestRuntimeException is really necessary/useful versus
>> just using RuntimeException, but that's a different issue.
>>
>> Thanks,
>> David
>>
>> On 8/12/2019 2:30 pm, serguei.spitsyn@oracle.com wrote:
>>> Hi Leonid,
>>>
>>> The fix looks good.
>>>
>>> Thank you for taking care about it!
>>> I agree, it is an awful duplication.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 12/7/19 18:17, Leonid Mesnik wrote:
>>>> Hi
>>>>
>>>> Could you please review following fix which just remove duplicated
>>>> threadByName methods and JDITestRuntimeException exceptions in
>>>> nsk/jdi tests. I don't see any reason to have so many copies of them.
>>>>
>>>> The method threadByName is added nsk.share.jdi.Debugee class as
>>>> 'threadByNameOrThrow' because slightly different 'threadByName'
>>>> already exist there. I filed another sub-task
>>>> https://bugs.openjdk.java.net/browse/JDK-8235544 to review usage
>>>> and merge these 2 methods later.
>>>>
>>>> This fix affects about ~4000 lines and I want to keep it as
>>>> straight-forward as possible.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~lmesnik/8235530/webrev.00/
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8235530
>>>>
>>>> The next planned steps are in:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8233830
>>>>
>>>> Leonid
>>>>
>>>
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
Very nice!<br>
<br>
<table style="color: rgb(0, 0, 0); font-family: Times; font-size:
12.8px; font-style: normal; font-variant-ligatures: normal;
font-variant-caps: normal; font-weight: 400; letter-spacing:
normal; orphans: 2; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: 2;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(238, 238, 238); text-decoration-style:
initial; text-decoration-color: initial;">
<tbody>
<tr>
<td>4153 lines changed: 21 ins; 3841 del; 291 mod; 99816 unchg</td>
</tr>
<tr>
</tr>
</tbody>
</table>
<br class="Apple-interchange-newline">
<br>
Coleen<br>
<br>
<div class="moz-cite-prefix">On 12/9/19 3:56 PM, Leonid Mesnik
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:2218fd4b-5070-aa83-2584-82f26187cd23@oracle.com">David,
Serguei
<br>
<br>
Thank you for review. I added comment about
JDITestRuntimeException in
<a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8235544">https://bugs.openjdk.java.net/browse/JDK-8235544</a>
<br>
<br>
Leonid
<br>
<br>
On 12/7/19 9:19 PM, David Holmes wrote:
<br>
<blockquote type="cite">+1 on both counts
<br>
<br>
Not sure JDITestRuntimeException is really necessary/useful
versus just using RuntimeException, but that's a different
issue.
<br>
<br>
Thanks,
<br>
David
<br>
<br>
On 8/12/2019 2:30 pm, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote: <br>
<blockquote type="cite">Hi Leonid,
<br>
<br>
The fix looks good.
<br>
<br>
Thank you for taking care about it!
<br>
I agree, it is an awful duplication.
<br>
<br>
Thanks,
<br>
Serguei
<br>
<br>
<br>
On 12/7/19 18:17, Leonid Mesnik wrote:
<br>
<blockquote type="cite">Hi
<br>
<br>
Could you please review following fix which just remove
duplicated threadByName methods and JDITestRuntimeException
exceptions in nsk/jdi tests. I don't see any reason to have
so many copies of them.
<br>
<br>
The method threadByName is added nsk.share.jdi.Debugee class
as 'threadByNameOrThrow' because slightly different
'threadByName' already exist there. I filed another sub-task
<a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8235544">https://bugs.openjdk.java.net/browse/JDK-8235544</a> \
to review usage and merge these 2 methods later.
<br>
<br>
This fix affects about ~4000 lines and I want to keep it as
straight-forward as possible.
<br>
<br>
webrev:
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8235530/webrev.00/">http://cr.openjdk.java.net/~lmesnik/8235530/webrev.00/</a>
<br>
<br>
bug: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8235530">https://bugs.openjdk.java.net/browse/JDK-8235530</a>
<br>
<br>
The next planned steps are in:
<br>
<br>
<a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8233830">https://bugs.openjdk.java.net/browse/JDK-8233830</a>
<br>
<br>
Leonid
<br>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic