JDK 8 code review request for 6380161 (reflect) Exception from newInstance() not chained to cause.

Sebastian Sickelmann sebastian.sickelmann at gmx.de
Wed Aug 10 20:03:13 UTC 2011


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.sickelmann at 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
>>
>




More information about the core-libs-dev mailing list