Hi Joel,

FYI, ccc request now approved; thanks,

-Joe

On 12/16/2015 1:15 AM, Joel Borggrén-Franck wrote:
Sounds good.

I'll ping someone in my time zone to check the status of the ccc and push once it is approved.

Thanks for the review

Cheers
/Joel
On mån 14 dec. 2015 at 20:45 joe darcy <joe.da...@oracle.com <mailto:joe.da...@oracle.com>> wrote:

    Hi Joel,

    Revised ccc request finalized for current version of the spec;
    should be approved in a day or two.

    Once that is approved, I think the current version can be pushed,
    enabling the refactorings you've alluded to to occur later.

    Thanks,

    -Joe


    On 12/13/2015 12:26 PM, Joel Borggrén-Franck wrote:
    Hi Joe,

    Thanks for the comments,

    On Thu, 10 Dec 2015 at 22:18 joe darcy <joe.da...@oracle.com
    <mailto:joe.da...@oracle.com>> wrote:

        Hi Joel,

        On 12/10/2015 12:27 PM, Joel Borggrén-Franck wrote:
        Question, is it better to remove the throws clauses for the
        cases that return null?

        I think so; they aren't applicable in those case and it is
        fine to remove exceptions in subtypes of course.


    Done.

        Please also add @Override annotations to the methods in the
        subtypes as a check that a new method is not accidentally
        being declared.


    Doh! Fixed.


        New webrev:
        http://cr.openjdk.java.net/~jfranck/8057804/webrev.02/
        <http://cr.openjdk.java.net/%7Ejfranck/8057804/webrev.02/>
        Diff of patch 01 and patch 02 (a diff-diff):
        http://cr.openjdk.java.net/~jfranck/8057804/diff_v1-v2.patch
        <http://cr.openjdk.java.net/%7Ejfranck/8057804/diff_v1-v2.patch>



        Please add an @implSpec note in AnnotatedType saying that
        "this implementation returns null", or words to that effect.


    Done.

        Shouldn't some of the implementation overrides in
        AnnotatedTypeFactory.java which throw null now be removed?
        Ah, I see the BaseImpl type is in the way. Is there an easy
        way to refactor that?


    I think it deserves a separate commit. I have another bugfix
    lined up, then I plan to refactor refactor the code a bit and
    clean up and improve testing.

        (I'll take care of the ccc changes once the new spec is
        finalized.)


    Thanks!

    New webrev:
    http://cr.openjdk.java.net/~jfranck/8057804/webrev.03/
    <http://cr.openjdk.java.net/%7Ejfranck/8057804/webrev.03/>
    Delta vs 01:
    http://cr.openjdk.java.net/~jfranck/8057804/diff_v1-v3.patch
    <http://cr.openjdk.java.net/%7Ejfranck/8057804/diff_v1-v3.patch>

    cheers
    /Joel


Reply via email to