vrozov commented on code in PR #52978:
URL: https://github.com/apache/spark/pull/52978#discussion_r2583409059
##########
project/SparkBuild.scala:
##########
@@ -1177,12 +1178,33 @@ object KubernetesIntegrationTests {
)
}
+object DependencyVersions {
Review Comment:
> code refactoring suggestions were almost ignored, I'd like to see the
technical explanation when doing that.
There were too many changes in the code refactoring suggestion, and not
every intention was clear. It would be much easier to explain what you wanted
to change instead of providing a single code suggestion.
> rename var `jacksonModuleID` => `jacksonBom`
It is not BOM, it is moduleID. BOM is the content of the file: "a BOM is a
special type of POM file that lists a curated set of dependencies and their
compatible versions. It's designed to simplify dependency management in
projects that use multiple modules from the same ecosystem". ModuleID is a way
to refer to pom, bom, and other artifacts using groupId, artifactId, and
version. The best I can do is rename `jacksonModuleID` => `jacksonBomModuleID`
(similar to `pomProperties` naming).
> merge getVersion and getVersionFromPom to getProperty - it's
overengineering, also disturbs IDE search functionality, when something goes
wrong, or users want to understand where fasterxml.jackson.version is used,
it's likely to do a global search
I tried your suggestion, and it does not work. IDE (IntelliJ) does not
understand that pom property is used in SBT build anyway. The expectation that
a string is always used as is is not a valid expectation. The goal here is to
make `DependencyVersions` object to support other artifacts in the following
PRs, and the current approach allows to avoid repeating ".version" over and
over again.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
