You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2023/03/03 10:00:08 UTC

[GitHub] [iceberg] nastra opened a new pull request, #6999: Build: Show API Deprecation rules on RevAPI failure

nastra opened a new pull request, #6999:
URL: https://github.com/apache/iceberg/pull/6999

   The main reasoning behind this change is to make people more aware of the API guarantees that the Iceberg project defined as part of https://github.com/apache/iceberg/blob/master/CONTRIBUTING.md#semantic-versioning.
   
   Once RevAPI fails, we also point people to the API Deprecation rules via the `showDeprecationRulesOnRevApiFailure` task. 
   
   Below is an example of the output that users would see (I've introduced an artificial API break so that RevAPI complains about it):
    
   ```
   ./gradlew revapi                                                                                                                                                                                                                                             1 ↵ ──(Fri,Mar03)─┘
   > Task :iceberg-api:revapi FAILED
   > Task :iceberg-api:showDeprecationRulesOnRevApiFailure FAILED
   
   FAILURE: Build completed with 2 failures.
   
   1: Task failed with an exception.
   -----------
   * What went wrong:
   Execution failed for task ':iceberg-api:revapi'.
   > There were Java public API/ABI breaks reported by revapi:
     
     java.method.addedToInterface: Method was added to an interface.
     
     old: <none>
     new: method void org.apache.iceberg.catalog.SessionCatalog::createTransaction()
     
     SOURCE: BREAKING, BINARY: NON_BREAKING, SEMANTIC: POTENTIALLY_BREAKING
     
     From old archive: <none>
     From new archive: iceberg-api-1.2.0-SNAPSHOT.jar
     
     If this is an acceptable break that will not harm your users, you can ignore it in future runs like so for:
     
       * Just this break:
           ./gradlew :iceberg-api:revapiAcceptBreak --justification "{why this break is ok}" \
             --code "java.method.addedToInterface" \
             --new "method void org.apache.iceberg.catalog.SessionCatalog::createTransaction()"
       * All breaks in this project:
           ./gradlew :iceberg-api:revapiAcceptAllBreaks --justification "{why this break is ok}"
       * All breaks in all projects:
           ./gradlew revapiAcceptAllBreaks --justification "{why this break is ok}"
     ----------------------------------------------------------------------------------------------------
   
   
   * Try:
   > Run with --stacktrace option to get the stack trace.
   > Run with --info or --debug option to get more log output.
   > Run with --scan to get full insights.
   ==============================================================================
   
   2: Task failed with an exception.
   -----------
   * Where:
   Build file '/home/nastra/Development/workspace/iceberg/build.gradle' line: 131
   
   * What went wrong:
   Execution failed for task ':iceberg-api:showDeprecationRulesOnRevApiFailure'.
   > ==================================================================================
     API/ABI breaks detected.
     Adding RevAPI breaks should only be done after going through a deprecation cycle.
     Please make sure to follow the deprecation rules defined in
     https://github.com/apache/iceberg/blob/master/CONTRIBUTING.md#semantic-versioning.
     ==================================================================================
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1125348566


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   Not necessarily. Local compile `gradle clean build -x intTest -x test` also executes revAPI.  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1124825995


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   yes. I didn't explicitly mention it thinking it was obvious and we will add `String.format()` while adding `%n`.
   
   There are other alternatives like `System.lineSeparator()` but that needs concat. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1124825995


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   yes. I didn't explicitly mention it thinking it was obvious.   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1125052343


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   This is okay since we're primarily using it in the CI environment right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1124735686


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   nit: Should we use platform independent carriage return like `%n` maybe instead of '\n'



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue merged pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue merged PR #6999:
URL: https://github.com/apache/iceberg/pull/6999


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1124819013


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   Doesn't %n only work in String.Format?
   ```scala
   scala> "Hello %n world"
   res6: String = Hello %n world
   
   scala> String.format("Hello %n world")
   res7: String =
   Hello
    world
    ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6999: Build: Show API Deprecation rules on RevAPI failure

Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #6999:
URL: https://github.com/apache/iceberg/pull/6999#discussion_r1124819013


##########
build.gradle:
##########
@@ -125,6 +125,26 @@ subprojects {
       oldName = project.name
       oldVersion = "1.1.0"
     }
+
+    tasks.register('showDeprecationRulesOnRevApiFailure') {
+      doLast {
+        throw new RuntimeException("==================================================================================" +

Review Comment:
   Doesn't %n only work in String.Format?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org