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