[GitHub] groovy pull request #617: GROOVY-8352

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
GitHub user aalmiray opened a pull request:

    https://github.com/apache/groovy/pull/617

    GROOVY-8352

    See https://issues.apache.org/jira/browse/GROOVY-8352
   
    Retention policy is set to `RUNTIME`in order to allow frameworks and libraries to have access to the new metadata.
   
    There's a breaking change in `Verifier`, a protected method now returns `MethodNode` instead of `void`. This change was reviewed with @blackdrag and deemed the better solution among alternatives (such as duplicate method & deprecate old).
   
    The JaCoCo team has done their part to prepare their project for this new annotation.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aalmiray/groovy at_generated_annotation

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/groovy/pull/617.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #617
   
----
commit 2e36707b9446f8b48a85bee26f6ec746d56e8b95
Author: aalmiray <[hidden email]>
Date:   2017-10-11T14:34:27Z

    GROOVY-8352: add a @Generated annotation

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
Github user marchof commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144137142
 
    --- Diff: src/main/groovy/transform/Generated.java ---
    @@ -0,0 +1,36 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package groovy.transform;
    +
    +import java.lang.annotation.ElementType;
    +import java.lang.annotation.Retention;
    +import java.lang.annotation.RetentionPolicy;
    +import java.lang.annotation.Target;
    +
    +/**
    + * The Generated annotation is used to mark members that have been generated.
    + *
    + * @author Andres Almiray
    + * @author Jochen Theodorou
    + * @author Mark Hoffmann
    --- End diff --
   
    It's "Marc Hoffmann" 😉


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144158718
 
    --- Diff: src/main/groovy/transform/Generated.java ---
    @@ -0,0 +1,36 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package groovy.transform;
    +
    +import java.lang.annotation.ElementType;
    +import java.lang.annotation.Retention;
    +import java.lang.annotation.RetentionPolicy;
    +import java.lang.annotation.Target;
    +
    +/**
    + * The Generated annotation is used to mark members that have been generated.
    + *
    + * @author Andres Almiray
    + * @author Jochen Theodorou
    + * @author Mark Hoffmann
    --- End diff --
   
    Actually, we are mostly trying not to use @author tags these days - though we haven't made it mandatory.
   
    Apache encourage removal of @author tags. It's not a mandated rule but they err on the side of wanting the whole community to own the whole codebase rather than people being put off by not wanting to touch a file "owned" by someone else. Git will tell us who the contributor was on a more fine-grained and accurate level than such tags anyway but obviously won't capture multiple authors committing under one of those names. Make sure anyone who has made a significant contribution is added into the `pomconfigurer.gradle` file.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user marchof commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144205553
 
    --- Diff: src/main/groovy/transform/Generated.java ---
    @@ -0,0 +1,36 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package groovy.transform;
    +
    +import java.lang.annotation.ElementType;
    +import java.lang.annotation.Retention;
    +import java.lang.annotation.RetentionPolicy;
    +import java.lang.annotation.Target;
    +
    +/**
    + * The Generated annotation is used to mark members that have been generated.
    + *
    + * @author Andres Almiray
    + * @author Jochen Theodorou
    + * @author Mark Hoffmann
    --- End diff --
   
    @paulk-asert I'm completely o.k. with removing my name here at all.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144216960
 
    --- Diff: src/main/groovy/transform/Generated.java ---
    @@ -0,0 +1,36 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package groovy.transform;
    +
    +import java.lang.annotation.ElementType;
    +import java.lang.annotation.Retention;
    +import java.lang.annotation.RetentionPolicy;
    +import java.lang.annotation.Target;
    +
    +/**
    + * The Generated annotation is used to mark members that have been generated.
    + *
    + * @author Andres Almiray
    + * @author Jochen Theodorou
    + * @author Mark Hoffmann
    --- End diff --
   
    @marchof the pomconfigurer.gradle file is a good option too Marc! :-)


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user aalmiray commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144219797
 
    --- Diff: src/main/groovy/transform/Generated.java ---
    @@ -0,0 +1,36 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package groovy.transform;
    +
    +import java.lang.annotation.ElementType;
    +import java.lang.annotation.Retention;
    +import java.lang.annotation.RetentionPolicy;
    +import java.lang.annotation.Target;
    +
    +/**
    + * The Generated annotation is used to mark members that have been generated.
    + *
    + * @author Andres Almiray
    + * @author Jochen Theodorou
    + * @author Mark Hoffmann
    --- End diff --
   
    updated PR by removing @author tags


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144309069
 
    --- Diff: src/main/org/codehaus/groovy/classgen/Verifier.java ---
    @@ -384,6 +387,7 @@ public void visit(MethodVisitor mv) {
         protected void addGroovyObjectInterfaceAndMethods(ClassNode node, final String classInternalName) {
             if (!node.isDerivedFromGroovyObject()) node.addInterface(ClassHelper.make(GroovyObject.class));
             FieldNode metaClassField = getMetaClassField(node);
    +        AnnotationNode generatedAnnotation = new AnnotationNode(ClassHelper.make(GENERATED_ANNOTATION));
     
    --- End diff --
   
    I needed a guard on adding the annotation to keep the build happy:
    ```
    boolean shouldAnnotate = classNode.getModule().getContext() != null;
    ```


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user aalmiray commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/617#discussion_r144314040
 
    --- Diff: src/main/org/codehaus/groovy/classgen/Verifier.java ---
    @@ -384,6 +387,7 @@ public void visit(MethodVisitor mv) {
         protected void addGroovyObjectInterfaceAndMethods(ClassNode node, final String classInternalName) {
             if (!node.isDerivedFromGroovyObject()) node.addInterface(ClassHelper.make(GroovyObject.class));
             FieldNode metaClassField = getMetaClassField(node);
    +        AnnotationNode generatedAnnotation = new AnnotationNode(ClassHelper.make(GENERATED_ANNOTATION));
     
    --- End diff --
   
    2 small fixes added 😄


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #617: GROOVY-8352

aalmiray-3
In reply to this post by aalmiray-3
Github user asfgit closed the pull request at:

    https://github.com/apache/groovy/pull/617


---