You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ConeyLiu (via GitHub)" <gi...@apache.org> on 2025/12/08 12:25:32 UTC
[PR] Follow-up: Apply spotless for scala code [iceberg]
ConeyLiu opened a new pull request, #14798:
URL: https://github.com/apache/iceberg/pull/14798
Address comment: https://github.com/apache/iceberg/pull/8023#discussion_r2580852812
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2600853323
##########
baseline.gradle:
##########
@@ -63,17 +63,19 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
+ String scalafmtConfigFile = null
// Configure different scalafmt rules for specific Scala version
- if (project.name.startsWith("iceberg-spark") && project.name.endsWith("2.13")) {
Review Comment:
Keep only applied to `iceberg-spark`
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2605031694
##########
baseline.gradle:
##########
@@ -182,7 +186,6 @@ subprojects {
}
pluginManager.withPlugin('scala') {
- String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
reviewer note, this is removed and now uses `scalaVersion` variable on L55
##########
baseline.gradle:
##########
@@ -63,18 +64,21 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
- // Configure different scalafmt rules for specific Scala version
- if (project.name.startsWith("iceberg-spark") && project.name.endsWith("2.13")) {
- scala {
- target 'src/**/*.scala'
- scalafmt("3.9.7").configFile("$rootDir/.baseline/scala/.scala213fmt.conf")
- licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt", "package"
+ String scalafmtConfigFile = null
+ if (project.name.startsWith("iceberg-spark")) {
Review Comment:
```suggestion
if (project.name.startsWith("iceberg-spark")) {
String scalafmtConfigFile = null
```
##########
baseline.gradle:
##########
@@ -52,6 +52,7 @@ subprojects {
apply plugin: 'com.palantir.baseline-exact-dependencies'
apply plugin: 'com.diffplug.spotless'
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
lets keep the original for consistency with other parts of the codebase
--
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
Re: [PR] Follow-up: Apply spotless for scala code [iceberg]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#issuecomment-3626672059
cc @jackylee-ch @kevinjqliu
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2604780015
##########
baseline.gradle:
##########
@@ -63,18 +64,21 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
- // Configure different scalafmt rules for specific Scala version
- if (project.name.startsWith("iceberg-spark") && project.name.endsWith("2.13")) {
- scala {
- target 'src/**/*.scala'
- scalafmt("3.9.7").configFile("$rootDir/.baseline/scala/.scala213fmt.conf")
- licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt", "package"
+ String scalafmtConfigFile = null
Review Comment:
How about moving this variable under `if (project.name.startsWith("iceberg-spark")) {`?
##########
baseline.gradle:
##########
@@ -52,6 +52,7 @@ subprojects {
apply plugin: 'com.palantir.baseline-exact-dependencies'
apply plugin: 'com.diffplug.spotless'
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
nit: We could call `getProperty(String key, String def)` instead `System.getProperty("scalaVersion", System.getProperty("defaultScalaVersion"))`. Feel free to ignore this comment as pre-existing.
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#issuecomment-3640453574
Thanks @kevinjqliu @jackylee-ch @ebyhr @huaxingao
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2599584617
##########
baseline.gradle:
##########
@@ -63,17 +63,19 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
+ String scalafmtConfigFile = null
// Configure different scalafmt rules for specific Scala version
- if (project.name.startsWith("iceberg-spark") && project.name.endsWith("2.13")) {
Review Comment:
do we need to keep this gate? `project.name.startsWith("iceberg-spark")`?
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#issuecomment-3639410708
Thanks for the refactor @ConeyLiu and thanks everyone for the review
--
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
Re: [PR] Follow-up: Apply spotless for scala code [iceberg]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2598484697
##########
baseline.gradle:
##########
@@ -63,17 +63,19 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
Can we move this out of the plugin so we can reuse it for [Scala unused checks](https://github.com/apache/iceberg/blob/main/baseline.gradle#L185).
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2599574150
##########
baseline.gradle:
##########
@@ -63,17 +63,19 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
btw its also referenced on L187
https://github.com/apache/iceberg/blob/9bfcdacfd3fdf31b4a350fa16204e8a9f01d7024/baseline.gradle#L187
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2604769862
##########
baseline.gradle:
##########
@@ -52,6 +52,7 @@ subprojects {
apply plugin: 'com.palantir.baseline-exact-dependencies'
apply plugin: 'com.diffplug.spotless'
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
nit: We could call `getProperty(String key, String def)` instead, `System.getProperty("scalaVersion", System.getProperty("defaultScalaVersion"))`. Feel free to ignore this comment as pre-existing.
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #14798:
URL: https://github.com/apache/iceberg/pull/14798#discussion_r2600847964
##########
baseline.gradle:
##########
@@ -63,17 +63,19 @@ subprojects {
licenseHeaderFile "$rootDir/.baseline/copyright/copyright-header-java.txt"
}
+ String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
Review Comment:
Updated
--
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
Re: [PR] Build: Improvements around applying spotless for Scala [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #14798:
URL: https://github.com/apache/iceberg/pull/14798
--
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