Nir Lisker wrote:

    As I understand it, you have added this as a possible refactoring
    for BooleanConditionBuilder (but left the original in for
    comparison), right?


Yes. I took Boolean as an example to demonstrate the approach.

    Since this would constitute a public API change, I don't think it
    should be done as part of this RFE.


I'm not sure what that change is. Is it extending the private ConditionBuilder class?

Yes. This becomes part of the public API at that point. Also, it generally isn't a good practice for a public class to extend non-public classes, so that would need to be considered.

-- Kevin


    It will be a couple days before I can look at the rest.


No problem.

-Nir On Tue, Mar 13, 2018 at 5:42 PM, Kevin Rushforth <kevin.rushfo...@oracle.com <mailto:kevin.rushfo...@oracle.com>> wrote:

    I took a quick look and had one comment:

       public class BooleanConditionBuilder2 extends
    ConditionBuilder<Boolean, BooleanBinding> { ... }

    As I understand it, you have added this as a possible refactoring
    for BooleanConditionBuilder (but left the original in for
    comparison), right? Since this would constitute a public API
    change, I don't think it should be done as part of this RFE.
    Otherwise, it becomes more than just a behavior-neutral
    implementation refactoring, and would need to be looked at as an
    API change, with all that entails.

    It will be a couple days before I can look at the rest.

    -- Kevin



    Nir Lisker wrote:

        Hi,

        Please review preliminary fix for:

        https://bugs.openjdk.java.net/browse/JDK-8199514
        <https://bugs.openjdk.java.net/browse/JDK-8199514>
        http://cr.openjdk.java.net/~nlisker/8199514/webrev.00/
        <http://cr.openjdk.java.net/%7Enlisker/8199514/webrev.00/>

        Thanks,
        Nir

Reply via email to