Repository navigation
Switch to scala 2.12 as build default - #3132
Conversation
|
The build fails because of this scala-garden/mima#422 the workaround is resolvers += Resolver.url(
"typesafe sbt-plugins",
url("https://dl.bintray.com/typesafe/sbt-plugins")
)(Resolver.ivyStylePatterns) |
2a1cf9f to
db18201
Compare
- geomesa does not cross publish for 2.12 - geowave does not corss publish for 2.12 These projects are both experimental so they're bing unlinked from the build until we either update or figure out the best way to deal with them.
Two cases here: - Type constraint was added for clarity but is actual constraint on the match - We know the numerical domain of the match in internal API
move commonSettings to Settings Use Def.settings
6ccc9af to
9ba482b
Compare
|
I requested a PMC approval, but not sure would it be completed successfully or not, due to ongoing changes in the locationtech eclipse structure. |
pomadchin
left a comment
pomadchin
left a comment
There was a problem hiding this comment.
LGTM, added a couple of comments to discuss, but it is good to go as it is. Also all CQs were approved by Jim and awaiting the IP team review. Merging once all CQs are closed!
| } | ||
|
|
||
| def monocle(module: String) = Def.setting { | ||
| "com.github.julien-truffaut" %% s"monocle-$module" % ver("1.5.1-cats", "2.0.0").value |
There was a problem hiding this comment.
I think RasterFrames are using the syntax like this?
There was a problem hiding this comment.
That's where I lifted this approach but I've certainly seen it in other projects. What do you think? I'm in favor of collapsing the version numbers to here so there is a minimum amount of indirection in the build overall.
| "project vectortile" test \ | ||
| "project util" test \ | ||
| "project gdal" test \ | ||
| "project geowave" compile test:compile \ |
There was a problem hiding this comment.
I hope we won't forget to remove the code from the codebase.
|
|
||
| val maxDistance = tileRDD.map(_._2.findMinMaxDouble).collect.foldLeft(-1.0/0.0){ (max, minMax) => scala.math.max(max, minMax._2) } | ||
| val cm = ColorMap((0.0 to maxDistance by (maxDistance/512)).toArray, ColorRamps.BlueToRed) | ||
| val cm = ColorMap(Range.BigDecimal.inclusive(0.0, maxDistance, maxDistance/512).map(_.toDouble).toArray, ColorRamps.BlueToRed) |
There was a problem hiding this comment.
Oy ... I know. I get what they're saying with this deprecation but this is the least-worst alternative. I want to keep the number of warnings down so we don't go into auto-ignore warning mode and miss something that matters though.
| ) | ||
| } | ||
|
|
||
| implicit val voxelKeyEncoder: Encoder[VoxelKey] = deriveEncoder[VoxelKey] |
There was a problem hiding this comment.
Hm, would the annotation here more user friendly? (less lines ~ easier to understand)
There was a problem hiding this comment.
They didn't compile for some reason and this was the work-around. I didn't look into why they didn't because it seems like the annotation has been very fragile in other cases and using deriveEncoder is mostly the default in my mind.
| targetHistograms: Seq[Histogram[T2]] | ||
| ): RDD[(K, MultibandTile)] = | ||
| rdd.map({ case (key, tile: MultibandTile) => | ||
| rdd.map({ case (key, tile) => |
There was a problem hiding this comment.
You don't need extra brackets here: rdd.map { case (key, tile) => (key, HistogramMatching(tile, sourceHistograms, targetHistograms)) }. This comment can be applied to all changes below btw.
There was a problem hiding this comment.
I'm going for minimum code change outside of configuration here, would rather not get sucked into format shuffling. Having said that once we get the guild and publishing sorted out we really should look into enforcing a code formatting standard for the library.
| @@ -0,0 +1 @@ | |||
| ThisBuild / version := "3.1.1-SNAPSHOT" | |||
There was a problem hiding this comment.
Is keeping the version in a separate file more convenient? With my gt-contrib and gt-server experiences I didn't notice any real wins ):
There was a problem hiding this comment.
Opening the discussion with this. Felt like I should pull this out of Version.scala which always stumps me. I'd be also OK with putting this version into build.sbt with other build settings.
There was a problem hiding this comment.
Let’s try with a version in a file; but I’m just worrying that it would be just an extra file and the story would be the same as a Versions class in a separate file.
2 participants
Switching to scala 2.12 gives us 25% faster build time on this project and better compatibility with tools like metals. Developer time is precious so that's nothing to sneeze at.
There are couple of issues with this PR that are still outstanding
geowaveversion we're using is 0.9.3 and has a strict dependency on scala 2.11 dependenciesgeomesaversion has similar problemOverall the geowave and geomesa projects are being unlinked from the main build for a bit as those problems look like they'll be harder to solve and both of those projects are experimental and not in heavy use.
CQs required:
Fixes #3139