Hi Roger, > On 15 Feb 2018, at 17:00, Roger Riggs <roger.ri...@oracle.com> wrote: > > Hi Robin, > > Looks fine to me.
Thanks for reviewing! > (How is this tested?, Normal, exceptional, etc.) The tests are part of the (currently closed) JFR tests. Best regards, Robin > > Thanks, Roger > > > On 2/15/2018 8:35 AM, Robin Westberg wrote: >> Hi again, >> >> Here’s the (hopefully) final version: >> >> Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ >> <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.02/> >> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ >> <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01-02/> >> >> Best regards, >> Robin >> >>> On 14 Feb 2018, at 16:15, Robin Westberg <robin.westb...@oracle.com >>> <mailto:robin.westb...@oracle.com>> wrote: >>> >>> Hi Roger, >>> >>>> On 13 Feb 2018, at 16:17, Roger Riggs <roger.ri...@oracle.com >>>> <mailto:roger.ri...@oracle.com>> wrote: >>>> >>>> Hi Robin, >>>> >>>> It looks like the status argument to BeforeHalt is discarded in >>>> JVM_BeforeHalt >>>> and is not inserted into the event. >>>> That suggests it should be removed all the way back to Shutdown.beforeHalt. >>> >>> You are right, my thinking was that the interface wouldn’t need to be >>> changed if we decided to revisit the event in the future and add the status >>> code. But that is perhaps unlikely, so can certainly remove the argument >>> for now. >>> >>> Best regards, >>> Robin >>> >>>> >>>> Roger >>>> >>>> >>>> >>>> On 2/13/2018 9:59 AM, Robin Westberg wrote: >>>>> Hi Alan, >>>>> >>>>>> On 12 Feb 2018, at 09:02, Alan Bateman <alan.bate...@oracle.com >>>>>> <mailto:alan.bate...@oracle.com>> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 12/02/2018 07:07, David Holmes wrote: >>>>>>>> Okay, I’ve removed the code related to the status field, certainly >>>>>>>> makes the change a bit less intrusive. >>>>>>>> >>>>>>>> Updated webrev: >>>>>>>> http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ >>>>>>>> <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01/> >>>>>>>> Incremental: >>>>>>>> http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ >>>>>>>> <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.00-01/> >>>>>>> This looks much cleaner/neater to me - thanks. >>>>>>> >>>>>> The updates to Runtime/Shutdown seems okay. >>>>> Thanks for reviewing! >>>>> >>>>> Best regards, >>>>> Robin >>>>> >>>>>> -Alan >>>> >>> >> >
