Hi Daniel, Looks good. The only other alternative would be to use java.io.CharConversionException over IOException. We could even consider dropping the cause because the subclass of I/O exception would convey the same meaning.
Minor formatting issues with a missing space after the catch keyword and missing a tab ahead of 'props.load' Jason ---------------------------------------- > Date: Tue, 24 Mar 2015 16:06:47 +0100 > From: daniel.fu...@oracle.com > To: lance.ander...@oracle.com > Subject: Re: RFR: 8075810: LogManager.readConfiguration may throw > undocumented IllegalArgumentException > CC: core-libs-dev@openjdk.java.net > > Hi Lance, > > Thanks for the review! > > On 24/03/15 16:01, Lance Andersen wrote: >> Hi Daniel, >> >> This looks OK but I might suggest clarifying that the cause could be set >> in the javadoc. Now that being said, I am not sure how consistent we >> are across the JDK or just make the assumption people will check the >> cause if they desire. > > Hmmm. I have the feeling that the best place for that would be i the > release notes - rather than in the API doc (which reminds me I should > plan to add some release note text to the issue). > >> As far as backporting, unless it really needed, I would probably would >> not as a change such as this typically should be done as part of an >> errata/MR (due to change of behavior and it is not that big of an issue). > > Right, my thinking too. Thanks for sharing your opinion! > > best regards, > > -- daniel > >> >> Best >> Lance >> On Mar 24, 2015, at 10:42 AM, Daniel Fuchs <daniel.fu...@oracle.com >> <mailto:daniel.fu...@oracle.com>> wrote: >> >>> Hi, >>> >>> Please find below a fix for >>> >>> 8075810: LogManager.readConfiguration may throw >>> undocumented IllegalArgumentException >>> >>> https://bugs.openjdk.java.net/browse/JDK-8075810 >>> >>> The proposed fix is to catch the IllegalArgumentException and >>> wrap it in an IOException, since LogManager.readConfiguration >>> is specified to throw IOException "if there are problems reading >>> from the stream." >>> >>> http://cr.openjdk.java.net/~dfuchs/webrev_8075810/webrev.00/ >>> >>> Initial discussion around the issue can be seen here: >>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-March/032348.html >>> >>> Question for reviewers: I will log a CCC for this - but I am wondering >>> whether I should plan to backport the fix to earlier release? >>> >>> >>> best regards, >>> >>> -- daniel >> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| >> Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >> >> >> >
