You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/12/14 20:35:01 UTC

[GitHub] [iceberg] kbendick commented on a change in pull request #3743: build: apply publishing config after project evaluation

kbendick commented on a change in pull request #3743:
URL: https://github.com/apache/iceberg/pull/3743#discussion_r769018497



##########
File path: deploy.gradle
##########
@@ -24,105 +24,108 @@ if (project.hasProperty('release') && jdkVersion != '8') {
 subprojects {
   apply plugin: 'maven-publish'
   apply plugin: 'signing'
+  afterEvaluate {
 
-  task sourceJar(type: Jar, dependsOn: classes) {
-    classifier = 'sources'
-    from sourceSets.main.allSource
-    group 'build'
-  }
+    task sourceJar(type: Jar, dependsOn: classes) {
+      classifier = 'sources'
+      from sourceSets.main.allSource
+      group 'build'
+    }
 
-  task javadocJar(type: Jar, dependsOn: javadoc) {
-    classifier = 'javadoc'
-    from javadoc.destinationDir
-    group 'build'
-  }
+    task javadocJar(type: Jar, dependsOn: javadoc) {
+      classifier = 'javadoc'
+      from javadoc.destinationDir
+      group 'build'
+    }
 
-  task testJar(type: Jar) {
-    archiveClassifier = 'tests'
-    from sourceSets.test.output
-  }
+    task testJar(type: Jar) {
+      archiveClassifier = 'tests'
+      from sourceSets.test.output
+    }
 
-  artifacts {
-    archives sourceJar
-    archives javadocJar
-    archives testJar
-    testArtifacts testJar
-  }
+    artifacts {
+      archives sourceJar
+      archives javadocJar
+      archives testJar
+      testArtifacts testJar
+    }
 
-  // add LICENSE and NOTICE
-  [jar, sourceJar, javadocJar, testJar].each { task ->
-    task.from(rootDir) {
-      include 'LICENSE'
-      include 'NOTICE'
+    // add LICENSE and NOTICE
+    [jar, sourceJar, javadocJar, testJar].each { task ->

Review comment:
       Small nit / learning opportunity: There is a trailing white space after the `->`. I think that's what's making this diff look different than the other ones (that show that they are leading space changes only).
   
   Generally speaking, the smaller the diff change the easier it is for people to cherry-pick and backport into their own forks. It's probably not as big of a deal for `deploy.gradle`, but I thought I'd take this opportunity to let you know as you get more involved with the project. 😄 
   
   Usually, the goal is to simply keep the number of changes as small as possible so that if people `git cherry-pick`, they're less likely to have issues with the diff applying correctly. This file doesn't merit that level of scrutiny imho (though I currently don't maintain a fork).
   
   That said, there's probably no major reason to change it. In some of the existing stuff in the file, there's a terminal space afterwards and some there is not. So this has just been for knowledge sake (if you weren't already aware). 😄 




-- 
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