Thanks for the quick first review. I have created to new patches for it:


Some logistics:
- What is the best way to bring this into the real codebase? Patch by Patch, or some bigger Diff - How to integrate? Should we rebase our work every time and keep it actual to newest commits,
      or is it easy to merge later?
- @David: How do we adjust/sync our work? Should we sync via private email? I think syncing via mailling list is like creating spam. - I think we should create at least 3 patches for each Exception we work on.
        - First: correct Exception/Error-Class
        - Second: adjust uses of corrected class (side-effect-free)
- Third++: Make some further adjustments to uses of the corrected Exception, like plumbing causes in catches where actually is code like this
    try (XYZException e) { throw new CorrectedException(e.toString());}
to
    try (XYZException e) { throw new CorrectedException(e.toString(), e);}

I think the latter one(Third++) would sometimes lead to greater discussion.

-- Sebastian

Am 09.08.2011 22:46, schrieb Joe Darcy:
Sebastian Sickelmann wrote:
Hi David,

it would be nice to have some help on this.

I started fixing InternalError yesterday [1] and discovered that this job is for someone with much industrousness (hope the translation is somewhat correct).

First of all a quick look at [1] would be good.
And most imported someone of the commiters who want to sponsor this work.
Anyone interrested in supporting/guiding?

-Sebastian

[1] https://bugs.openjdk.java.net/attachment.cgi?id=229&action=diff)

Some comments on [1], first I assume all the modified files are in the "jdk" repository.

As a typographical nit, in something like

   ew InternalError(ex.getMessage(),ex);

there should be a space between the comma and "ex".

In the javadoc of InternalError, prefer "{@code InternalError}" over "<code>InternalError</code>".

In a line like this from URLClassPath

               throw (InternalError) new InternalError(e);

the cast is no longer needed.

In ProxyClient.java, the new code is missing a throw statement.

-Joe



Am 08.08.2011 23:28, schrieb David Schlosnagle:
On Mon, Aug 8, 2011 at 4:57 PM, Sebastian Sickelmann
<sebastian.sickelm...@gmx.de>  wrote:
These ones are candidates for cleanup too, but i want to discuss if someone
sees a good argument to not to change to
    throw new InternalError(e.getMessage(),e);
    or
    throw new InternalError("Lorem ipsum",e);

Also i don't completely understand why Throwable(Throwable cause) uses
cause.toString() to fill the message instead of cause.getMessage().
Can someone explain to me why?
If you look at Throwable.toString(), you'll see that it uses
Throwable.getLocalizedMessage() which allows for localized exception
messages, so I would avoid using getMessage() directly.

I'd also be interested in helping out making the Throwable and
Exception types provide the 4 consistent constructors if there are
committers that would help guide us.

- Dave



Reply via email to