On Apr 20, 2015, at 5:49 PM, Roger Riggs <[email protected]> wrote:
> Hi Paul,
>
> On 4/20/2015 9:01 AM, Paul Sandoz wrote:
>> Hi Roger,
>>
>> I am not sure you have the @implSpec/@implNote quite correct on the new
>> methods of Process.
>>
>> For example, for Process.toHandle i would expect something like:
>>
>> ...
>>
>> @implSpec
>> This implementation throws an instance of UnsupportedOperationException and
>> performs no other action. Sub-types should override this method, ensuring
>> that
>> calling methods (getPid etc.) of this class, that are not overridden,
>> operate on the
>> returned ProcessHandle.
>>
>> The @implSpec should refer to the implementation in Process itself, and the
>> @implNote cannot be used for any normative statements.
> Thanks for the reminder and suggested text. I updated the @implSpec clauses.
>
I spotted another one here:
281 /**
282 * Returns {@code true} if the implementation of {@link #destroy} is to
283 * normally terminate the process,
284 * Returns {@code false} if the implementation of {@code destroy}
285 * forcibly and immediately terminates the process.
286 *
287 * @implSpec
288 * Processes returned from ProcessBuilder support this operation to
289 * return true or false depending on the platform implementation.
290 *
291 * @return {@code true} if the implementation of {@link #destroy} is to
292 * normally terminate the process;
293 * otherwise, {@link #destroy} forcibly terminates the process
294 * @throws UnsupportedOperationException if the Process implementation
295 * does not support this operation
296 * @since 1.9
297 */
298 public boolean supportsNormalTermination() {
299 throw new UnsupportedOperationException("supportsNormalTermination
not supported for " + this.getClass().toString());
300 }
The docs of Process.onExit might also require some tweaks. I dunno how much
wiggle room there is with the current implementation, perhaps very little?
415 /**
416 * Returns a ProcessHandle for the Process.
417 *
418 * @implSpec
419 * This implementation throws an instance of
UnsupportedOperationException
420 * and performs no other action.
...
446 * @implSpec
447 * The implementation of this method returns information about the
process as:
448 * {@link #toHandle toHandle().info()}.
Best to be consistent with either "This implementation ..." or "The
implementation of this method ..." rather than a mix. I prefer the former as it
is more concise. Up to you.
> Some @implNotes describe the JDK implementation and some developers will
> rely on the implementation specifics and to some degree define the expected
> behaviors.
One way of thinking about this is that the developers writing the TCK tests
will pay close attention to @implSpec and need not do so for @implNote.
>>
>>
>> The document for Process.getPid (and similarly those methods depending on
>> toHandle) could then be:
>>
>> ...
>> @implSpec
>> This implementation returns a process id as follows:
>>
>> toHandle().getPid();
>>
>>
>> In this respect is there a need to say anything about the behaviour of a
>> Process created by ProcessBuilder?
> The ProcessBuilder produced subclasses of Process implement the spec
> so no additional description is needed.
>>
>> It might be useful to have some general guidance for sub-types on the class
>> doc of Process e.g. saying they only need to override toHandle but may
>> override some or all dependent methods as appropriate.
> It does not add much but I added a paragraph to the Process class javadoc.
> There are not many subclasses of Process outside the JDK.
>
Ok.
Paul.
> The webrev[1] and javadoc[2] have been updated in place.
>
> Thanks, Roger
>
> [1] http://cr.openjdk.java.net/~rriggs/webrev-ph
> [2] http://cr.openjdk.java.net/~rriggs/ph-apidraft/
