Re: RFR (M) 8229036: Remove the testing against NSK_TRUE from tests

Tue, 06 Aug 2019 09:04:12 -0700

Hi Jc,

It looks good.

Thanks,
Serguwi


On 8/6/19 08:49, Jean Christophe Beyler wrote:
Thanks Chris!

Could I get a second review please? :)
Jc

On Fri, Aug 2, 2019 at 4:20 PM Chris Plummer <chris.plum...@oracle.com> wrote:
Ok. I think the changes are fine as is.

thanks,

Chris

On 8/2/19 3:00 PM, Jean Christophe Beyler wrote:
Hi Chris,

I only did it when there were repercussions to the change of if (.* == NSK_TRUE).

For example:

I wanted to move:

-    if (success != NSK_TRUE) {
+    if (!success) {

But really I was thinking that success should be a bool then. However, success was assigned also by:
         success = checkTime(jvmti, &time, &prevTime, "VM_DEATH callback");
So I went to transform checkTime to return a boolean, and that rippled into changing the NSK_FALSE to false as well.
I can reduce the scope of this webrev to only being the if statement if you prefer, I was just working on getting the various elements to bool instead of int and NSK_TRUE/NSK_FALSE.
Another solution would be to maybe augment the scope of the bug item to : Move NSK_TRUE/NSK_FALSE to true/false; and this webrev as a side-effect covers all the "if (.* NSK_TRUE)" cases.
What do you think?
Jc


On Fri, Aug 2, 2019 at 2:40 PM Chris Plummer <chris.plum...@oracle.com> wrote:
Hi JC,

Why does this webrev also remove references NSK_FALSE, and the previous one references to NSK_TRUE?

thanks,

Chris

On 8/2/19 2:21 PM, Jean Christophe Beyler wrote:
Hi all,

Here is the webrev that does the removal of if (.* == NSK_TRUE) and replaces them with if (.*).


This was tested by running the tests changed on my dev machine, I'll push it to the submit repo after review :-)

Thanks and have a great day & weekend!,
Jc



--

Thanks,
Jc



--

Thanks,
Jc

Reply via email to