Hi,

This can't be pushed without a CSR approval for JDK10 and that process won't
be "up and running" for about another 10 days.

But before we get to that I wonder why we need to return Path2D when we
aren't creating a new one .. there doesn't seem to be any value to it and
it just potentially misleading a caller into thinking it has the cost of doing so.

If any value is worth returning it might be a boolean to indicate whether any trimming
actually occurred.

-phil.

On 4/20/17, 4:41 AM, Laurent Bourgès wrote:
Phil,

Do you approve the patch and the javadoc too ?

If yes, could you push it for me into OpenJDK10 ?

Laurent


2017-04-20 11:55 GMT+02:00 Prahalad Kumar Narayanan <prahalad.kumar.naraya...@oracle.com <mailto:prahalad.kumar.naraya...@oracle.com>>:

    Looks good to me.

    Thanks
    Have a good day

    Prahalad N.

    ----------------------------------------------------------------------

    From: Laurent Bourgès [mailto:bourges.laur...@gmail.com
    <mailto:bourges.laur...@gmail.com>]
    Sent: Thursday, April 20, 2017 11:51 AM
    To: Prahalad Kumar Narayanan
    Cc: 2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>
    Subject: Re: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage
    trimming

    Hello,
    Here is an updated webrev:
    http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.1/
    <http://cr.openjdk.java.net/%7Elbourges/path2D/Path2D-8078192.1/>
    I fixed the copyright dates and the javadoc as you proposed (I
    initially derived the javadoc from Vector.trimToSize)
    Thanks,
    Laurent

    2017-04-20 5:46 GMT+02:00 Prahalad Kumar Narayanan
    <prahalad.kumar.naraya...@oracle.com
    <mailto:prahalad.kumar.naraya...@oracle.com>>:
    Hello Laurent

    The code changes look good.
        . The new method trimToSize effectively does the same
    operation (Arrays.copyOf) as done by the copy contructors.
        . The test file- Path2DCopyConstructor now tests after
    executing the methods pathObj.trimToSize and pathObj.clone

    With regard to the text in Javadoc comments
        . I felt, the first line clearly explains the operation -
                   "Trims the capacity of this Path2D instance to its
    current size."
        . The immediate following line is redundant and may not be
    required -
                   "If the capacity .. is larger than its current
    size... "
        . The third line will be required till the end of the comments -
                   "An application can use ... minimize the storage of
    a path. ... since 10"
        . This is my observation. You could wait for other suggestions.

    Btw, the copyright year should be changed to 2017 in both
    Path2D.java and the test file.
    (Some use automation scripts that modify the year at the time of
    check-in. If so, kindly ignore the observation)

    Thanks
    Have a good day

    Prahalad N.


    ----------------------------------------------------------------------

    Message: 1
    Date: Wed, 19 Apr 2017 08:49:33 +0200
    From: Laurent Bourg?s <bourges.laur...@gmail.com
    <mailto:bourges.laur...@gmail.com>>
    To: "2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>"
    <2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>>, Phil Race
    <philip.r...@oracle.com <mailto:philip.r...@oracle.com>>,  Jim
    Graham <james.gra...@oracle.com <mailto:james.gra...@oracle.com>>,
    Iris
            Clark <iris.cl...@oracle.com <mailto:iris.cl...@oracle.com>>
    Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
    Message-ID:
    <CAKjRUT7zQ=zsqiqdbpdtoecosfolvpzqhdvi-pxzsmwtgkr...@mail.gmail.com 
<mailto:zsqiqdbpdtoecosfolvpzqhdvi-pxzsmwtgkr...@mail.gmail.com>>
    Content-Type: text/plain; charset="utf-8"

    Hi,

    Here is a first attempt to propose a Path2D patch (based on JDK10):
    http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/
    <http://cr.openjdk.java.net/%7Elbourges/path2D/Path2D-8078192.0/>

    JBS: https://bugs.openjdk.java.net/browse/JDK-8078192
    <https://bugs.openjdk.java.net/browse/JDK-8078192>

    Please review the Path2D changes, notably the javadoc (english)
    and the modified Path2DCopyConstructor test which checks all
    public Path2D methods on concrete classes (Path2D.Float,
    Path2D.Double, GeneralPath) after calling path.trimToSize()

    Cheers,
    Laurent



    --
    --
    Laurent Bourgès




--
--
Laurent Bourgès

Reply via email to