Looks good!

Thanks
Maurizio

On 10/06/16 16:58, ShinyaYoshida wrote:
Hi Maurizio,
Thank you for your review!

I've just updated the webrev:
http://cr.openjdk.java.net/~shinyafox/jigsaw/8158123/webrev.01/ <http://cr.openjdk.java.net/%7Eshinyafox/jigsaw/8158123/webrev.01/>

In this webrev, I've tried to check that the annotation is not permitted at any of the module names or the package names in module-info.
Please review this again.

2016-06-07 18:54 GMT+09:00 Maurizio Cimadamore <maurizio.cimadam...@oracle.com <mailto:maurizio.cimadam...@oracle.com>>:

    Hi,
    the patch looks good.

    I would add another test for a case like this:

    module m1 { exports p to m2.@Anno foo; }

    and, possibly, for other constructs in a module-info which accept
    module names.

    From a more general point of view, I note that the parser is using
    the qualified identifier production to get at a module name
    (probably because module names can contain dots?). I'll defer to
    Alex any clarification to how the module grammar is specified -
    but I note here that, implementation-wise, perhaps having another
    production-like method to parse module names could be a winning
    move - since I don't think module names have much to do with
    qualified identifiers.


So... Should I wait for the reply from Alex?

Regards,
shinyafox(Shinya Yoshida)



    Maurizio


    On 06/06/16 21:56, Maurizio Cimadamore wrote:
    Hi Shinya
    Jan is on vacation for few days, I'll try to look at it tomorrow.

    Thanks
    Maurizio

    On 06/06/16 14:16, ShinyaYoshida wrote:
    Hi Jan and Maurizio,
    Could you review this?

    webrev:
    http://cr.openjdk.java.net/~shinyafox/jigsaw/8158123/webrev/
    <http://cr.openjdk.java.net/%7Eshinyafox/jigsaw/8158123/webrev/>

    bugs:
    https://bugs.openjdk.java.net/browse/JDK-8158123

    Regards,
    shinyafox(Shinya Yoshida)

    2016-05-30 14:09 GMT+09:00 ShinyaYoshida <bitterf...@gmail.com
    <mailto:bitterf...@gmail.com>>:

        Hi jigsaw-dev,
        I've filed the issue and created the patch for it.

        Please review this:

        webrev:
        http://cr.openjdk.java.net/~shinyafox/jigsaw/8158123/webrev/
        <http://cr.openjdk.java.net/%7Eshinyafox/jigsaw/8158123/webrev/>

        bugs:
        https://bugs.openjdk.java.net/browse/JDK-8158123

        Regards,
        shinyafox(Shinya Yoshida)






Reply via email to