You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nikhils05 <gi...@git.apache.org> on 2014/05/21 11:43:22 UTC

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

GitHub user nikhils05 opened a pull request:

    https://github.com/apache/spark/pull/845

     [SPARK-1820][tools] Make GenerateMimaIgnore @DeveloperApi annotation aware.

    Solution for :  Add all the classes with DeveloperApi annotation in Mima excludes.

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

    $ git pull https://github.com/nikhils05/spark tools

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

    https://github.com/apache/spark/pull/845.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 #845
    
----
commit f495620d0f11e34dee67853dd0912fe20602d24d
Author: nikhil7sh <ni...@gmail.ccom>
Date:   2014-05-19T06:04:02Z

    (SPARK-1820) Make GenerateMimaIgnore @DeveloperApi annotation aware

commit 6a7201b3bdbf917ea0054049eeaded13bfcbfd72
Author: nikhil7sh <ni...@gmail.ccom>
Date:   2014-05-21T09:16:10Z

    [SPARK-1820] Make GenerateMimaIgnore @DeveloperApi annotation aware

commit 8fa02d2c67f16556d7477f515603d472d2679a21
Author: nikhil7sh <ni...@gmail.ccom>
Date:   2014-05-21T09:20:04Z

    [SPARK-1820] Make GenerateMimaIgnore @DeveloperApi annotation aware

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44371001
  
    Jenkins, test this please. Jenkins, this is okay to test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-54099382
  
    @nikhils05 Can you close this PR ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44226148
  
     Merged build triggered. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/845#discussion_r13033037
  
    --- Diff: tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala ---
    @@ -68,10 +70,26 @@ object GenerateMIMAIgnore {
               false
             }
           }
    -    }
    -
    +    }  
    +	
    +    def classesAnnotationCheck(className: String) = {
    +	try {
    --- End diff --
    
    Please convert tabs to spaces (otherwise indentation looks really wack on github).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/845#discussion_r13037266
  
    --- Diff: tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala ---
    @@ -69,9 +71,25 @@ object GenerateMIMAIgnore {
             }
           }
         }
    -
    +    
    +    def classesAnnotationCheck(className: String) = {
    +    try {
    +     val annotList=mirror
    +       .classSymbol(Class.forName(className, false, classLoader))
    +       .annotations
    +	
    +       isAnnotationExistClassLevel(annotList)
    +     } catch {
    +       case _: Throwable => {
    +          println("Error determining Annotations: " + className)
    +          false
    +          }
    +       }
    +     }  
    +	
         for (className <- classes) {
           val directlyPrivateSpark = isPackagePrivate(className)
    +      val annotationCheck = classesAnnotationCheck(className)
    --- End diff --
    
    It might be better to call this something like `val developerApi` to make it grammatically consistent with the other variables. I.e.
    ```
    if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi) privateClasses += className
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/845#discussion_r13037443
  
    --- Diff: tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala ---
    @@ -69,9 +71,25 @@ object GenerateMIMAIgnore {
             }
           }
         }
    -
    +    
    +    def classesAnnotationCheck(className: String) = {
    --- End diff --
    
    This could use some formatting and naming cleanup - the indentation is weird. Also, it might be better to inline `isAnnotationExistClassLevel` rather than having its own function.
    
    What about this?
    ```
    def isDeveloperApi(className: String) = {
      try {
        val clazz = mirror.classSymbol(Class.forName(className, false, classLoader))
        clazz.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.DeveloperApi])
      } catch {
        case _: Throwable => {
          println("Error determining Annotations: " + className)
          false
         }
       }
     }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by nikhils05 <gi...@git.apache.org>.
Github user nikhils05 commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44185771
  
    I've made the suggested changes...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44145808
  
    Merged build finished. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44371301
  
    Merged build finished. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-47295223
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44145809
  
    
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15189/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44371302
  
    
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15247/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44145756
  
     Merged build triggered. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44226201
  
    
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15214/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/845


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44400146
  
    #904 Fixes the build and few other style things. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-43734997
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44145728
  
    Jenkins, test this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44160938
  
    I added some style feedback. This is failing several style tests:
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15189/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44226152
  
    Merged build started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44371215
  
     Merged build triggered. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44145763
  
    Merged build started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44226200
  
    Merged build finished. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44226045
  
    Jenkins, test this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-54103378
  
    Yeah let's close this issue for now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-1820][tools] Make GenerateMimaIgnore @D...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/845#issuecomment-44371220
  
    Merged build started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---