RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

[Roger Riggs]

Hi Alan,

On 4/24/2015 12:21 PM, Alan Bateman wrote:
> On 24/04/2015 16:49, Roger Riggs wrote:
>> :
>>>
>>> I'm not sure about the @implSpec in 
>>> Process::supportsNormalTermination. Shouldn't that just specify that 
>>> the default implementation throws UOE. An @implNote could comment on 
>>> how ProcessBuilder works. Same comment on Process::toHandle.
>>>
>> There needs to be a specification for what ProcessBuilder does; 
>> @implNote cannot provide testable assertions.
> Right, I'm not suggesting that, just pointing out @implSpec in the 
> webrev doesn't specify how the default implementation works. It 
> instead specifies what ProcessBuilder#start returns. Is the javadoc 
> and webrev in sync, that might be part of the confusion on my part.
Right, Paul pointed out this as well and I got the javadoc out of sync 
with the webrev.
The javadoc was generated more recently.  I should have updated them at 
the same time or not at all.
>
>
...
>
>>
>>>
>>> The parent is optional and maybe it would make sense for parent() to 
>>> return Optional<ProcessHandle>. The javadoc for this method, isAlive 
>>> too, mention "zombie state". I think that needs a bit of description 
>>> to generally explain what it means.
>> Will providing Optional make it easier to use?
>>
>> Perhaps the mention of zombie should be removed; it is os specific; 
>> though the description of the state may be useful.
> If you keep it then some commentary would be helpful as the reader 
> might not understand the term.
will do.
>
> The parent is optional so something to consider.
I'm open to well reasoned recommendations.
>
>
>
>>>
>>> This may have been discussed previously but totalCpuDuration might 
>>> be clearer if renamed to totalCpuTimeAsDuration. Also startInstant 
>>> would be clearer (to me anyway) as startTimeAsInstant.
>> I would have preferred not include the return type at all instead of 
>> making the method name longer.
> Okay although not having "CpuTime" and "StartTime" in the method names 
> means the names will be less clear.
These started out as totalCputime() and startTime(). Those seemed clear 
enough to me.
When you suggest 'AsDuration' is the qualification to indicate the 
return type or
the general nature of a time interval.

Thanks, Roger

>
> -Alan