[cp-patches] RFC: New events for JDWP and small changes to Event internals

Keith Seitz keiths at redhat.com
Fri Jun 9 19:49:15 UTC 2006


Kyle Galloway wrote:
> ChangeLog:
> 2006-06-09   Kyle Galloway   <kgallowa at redhat.com>
> 
>    * gnu/classpath/jdwp/event/BreakpointEvent.java: Added Object
>    _instance for filter compatibility
>    * gnu/classpath/jdwp/event/BreakpointEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ClassPrepareEventEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/Event.java: Added constants for
>    type and changed the abstract method getParameters to match
>    * gnu/classpath/jdwp/event/ThreadEndEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ThreadStartEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/VmDeathEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/VmInitEvent.java(getParameter):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ClassMatchFilter.java(matches):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ClassOnlyFilter.java(matches):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/InstanceOnlyFilter.java(matches):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ThreadOnlyFilter.java(matches):
>    changed from Class type to constants
>    * gnu/classpath/jdwp/event/ExceptionEvent.java: New file

Need a space between filename and methods in changelog entries. 
<nitpick>Also a blank line between the last entry, since it is not 
related to those above it.</nitpick>

All the Event changes to getParameter are approved. [Boy, don't we wish 
we could finally use an enum!]

> Index: gnu/classpath/jdwp/event/ExceptionEvent.java
> ===================================================================
> RCS file: gnu/classpath/jdwp/event/ExceptionEvent.java
> diff -N gnu/classpath/jdwp/event/ExceptionEvent.java
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gnu/classpath/jdwp/event/ExceptionEvent.java	1 Jan 1970 00:00:00 -0000

[snip]

> +package gnu.classpath.jdwp.event;
> +
> +import java.io.DataOutputStream;
> +import java.io.IOException;
> +
> +import gnu.classpath.jdwp.JdwpConstants;
> +import gnu.classpath.jdwp.VMIdManager;
> +import gnu.classpath.jdwp.id.ThreadId;
> +import gnu.classpath.jdwp.util.Location;
> +import gnu.classpath.jdwp.id.ObjectId;

Okay, this is kinda nit-pickity, but it *is* in the classpath hacker's 
guide: ObjectId is out of alphabetical order.

> +  /**
> +   * Writes the event to the given stream
> +   * 
> +   * @param outStream the output stream to write the event to
> +   */

Missing @throws in javadoc. I'm probably very guilty of forgetting this 
a lot, too.

> +  protected void _writeData(DataOutputStream outStream) throws IOException

"throws" should be on next line.

Otherwise it looks good. Got a savannah account yet?


Keith



More information about the Classpath-patches mailing list