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