Thanks Vivek,

Looks good to me.

On 4/12/18 9:25 PM, Vivek Theeyarath wrote:

Hi Roger,

Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.04/ <http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.04/> . I fixed the indentation with my earlier fix (asPredicate) too. Hope that is fine.

Regards

Vivek

*From:*Roger Riggs
*Sent:* Thursday, April 12, 2018 7:43 PM
*To:* Vivek Theeyarath <vivek.theeyar...@oracle.com>
*Cc:* Core-Libs-Dev <core-libs-dev@openjdk.java.net>
*Subject:* Re: RFR: 8184692: add Pattern.asMatchPredicate

Hi Vivek,

ok,

Generally, I like to see an updated webrev so that an old non-final webrev isn't left around to be a source of questions. Its easy enough to create a simple script to create the webrev and copy it to cr.openjdk...

Continued javadoc @xxx lines should be indented to improve readability of the source.

5845      *           against this pattern.


Thanks, Roger

On 4/12/18 8:49 AM, Vivek Theeyarath wrote:

    Hi,

       I have updated the doc as per the suggestion. Have finalized the csr too.

    Regards

    Vivek

    -----Original Message-----

    From: Paul Sandoz

    Sent: Thursday, April 12, 2018 12:40 AM

    To: Vivek Theeyarath<vivek.theeyar...@oracle.com> 
<mailto:vivek.theeyar...@oracle.com>

    Cc: Roger Riggs<roger.ri...@oracle.com> <mailto:roger.ri...@oracle.com>; 
Core-Libs-Dev<core-libs-dev@openjdk.java.net>
    <mailto:core-libs-dev@openjdk.java.net>

    Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

    HI,

    Hopefully this is the last update :-)

    To align with asPredicate and Roger’s prior guidance on that method i think 
you can just say this:

       Creates a predicate that tests if this pattern matches a given input 
string.

    which is similar to the language of Pattern.matches.

    No need for another webrev if this change is acceptable.

    Paul.

        On Apr 9, 2018, at 7:53 PM, Vivek Theeyarath<vivek.theeyar...@oracle.com> 
<mailto:vivek.theeyar...@oracle.com>  wrote:

        Thanks Paul / Roger for the inputs.

        I have updated the javadoc based on the 
inputs.http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.03
        <http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.03>  . Please 
review.

        Regards

        Vivek

        -----Original Message-----

        From: Paul Sandoz

        Sent: Monday, April 09, 2018 9:25 PM

        To: Roger Riggs<roger.ri...@oracle.com> <mailto:roger.ri...@oracle.com>

        Cc: Core-Libs-Dev<core-libs-dev@openjdk.java.net>
        <mailto:core-libs-dev@openjdk.java.net>

        Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

            On Apr 9, 2018, at 8:41 AM, Roger Riggs<roger.ri...@oracle.com> 
<mailto:roger.ri...@oracle.com>  wrote:

            Hi Vivek,

            As with Pattern.asPredicate the first sentence can be improved.

              Creates a predicate that tests if this pattern matches the entire 
region.

        The region of what? region is clear from the context of a Matcher, but 
less so from the context of Pattern.

        Paul.

            5833: As with the original issue, perhaps adding the word 'whole' or

            'entire' will make it clearer that the pattern must match then 
entire input string.

            5827:  Split into two sentences, the second one starting  "For 
example,"

            5840: add a blank line between methods

            Regards, Roger

            On 4/9/18 5:05 AM, Vivek Theeyarath wrote:

                Hi,

                        Please find the updated webrev after incorporating 
Paul's comments.

                http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.02/
                <http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.02/>

                Also, I have created a csr for this 
issuehttps://bugs.openjdk.java.net/browse/JDK-8201308  .

                Regards

                Vivek

                -----Original Message-----

                From: Paul Sandoz

                Sent: Friday, April 06, 2018 6:55 AM

                To: Vivek Theeyarath<vivek.theeyar...@oracle.com>
                <mailto:vivek.theeyar...@oracle.com>

                Cc: Core-Libs-Dev<core-libs-dev@openjdk.java.net>
                <mailto:core-libs-dev@openjdk.java.net>

                Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

                    On Apr 4, 2018, at 10:47 AM, Vivek 
Theeyarath<vivek.theeyar...@oracle.com>
                    <mailto:vivek.theeyar...@oracle.com>  wrote:

                    Hi All,

                                  Please review.

                    Bug:https://bugs.openjdk.java.net/browse/JDK-8184692

                    Webrev 
:http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.00/
                    
<http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.00/>

                Like with your other patch, alignment to ~80 chars would be 
good, as that is mostly consistent with other code in the same source file.

                Let’s not use the word “find" here, so as not to confuse with 
matcher(s).find().

                5833      * @return  The predicate which can be used for 
finding if an input string matches this pattern.

                I suggest:

                @return  The predicate which can be used for matching an input

                string against this pattern

                You could also add a @see Matcher#matches

                Paul.

                    The related jtreg test was run and the test passed .

                    Regards

                    Vivek


Reply via email to