Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jdk11 compatibility and Gradle 5 #132

Merged
merged 12 commits into from
Mar 8, 2019
Merged

Jdk11 compatibility and Gradle 5 #132

merged 12 commits into from
Mar 8, 2019

Conversation

matthieun
Copy link
Contributor

@matthieun matthieun commented Mar 5, 2019

Description:

This enables the project to be built using OpenJDK 11. Followup of osmlab/atlas#366

This also enables gradle 5.2.1

  • Tested with OpenJDK 11.0.2+9
$ java -version
openjdk version "11.0.2" 2019-01-15
OpenJDK Runtime Environment 18.9 (build 11.0.2+9)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.2+9, mixed mode)
  • Updated atlas and atlas-generator dependencies
  • Spotless 3.18.0
    • importOrder([ x, y, z]) becomes importOrder x, y, z
  • Upgraded to latest guava (temporarily, until JDK11 Followup atlas#374 is merged and released). This solves issues with the classpath scanning utilities that stopped working in jdk11.
  • Fixed conflicts caused by Atlas GeoJson Interfaces atlas#369 and fixed related test failures.
  • shadow plugin 5.0.0
  • Gradle wrapper 5.2.1
  • Checkstyle 8.18
    • Some checks that were already enabled started working, so fixed the related issues.
  • gradle run updates
    • runGenerator.execute() is invalid. Replaced by run.finalizedBy(runGenerator) at the end.
  • Added maven central badge

Potential Impact:

The impact should be confined to this repository.

Unit Test Approach:

Ran unit tests and integration tests.

Test Results:

Tests pass

* {@link FlaggedRelation} feature
* @return geometry as {@link JsonElement}
*/
private static JsonElement getFlaggedRelationGeometryFromFeature(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ordering only

@matthieun matthieun marked this pull request as ready for review March 7, 2019 22:06
@matthieun matthieun changed the title Jdk11 compatibility Jdk11 compatibility and Gradle 5 Mar 8, 2019
{
final JsonObject feature = atlasItem.asGeoJsonGeometry();
final JsonObject properties = feature.getAsJsonObject("properties");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would return null and fail some tests after osmlab/atlas#369

{
feature = locationItem.asGeoJsonGeometry();
properties = feature.getAsJsonObject("properties");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would return null and fail some tests after osmlab/atlas#369

@@ -62,9 +44,10 @@ public FlaggedRelation(final Relation relation)
public JsonObject asGeoJsonFeature(final String flagIdentifier)
{
final JsonObject feature = this.relation.asGeoJsonGeometry();
final JsonObject featureProperties = feature.getAsJsonObject("properties");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would return null and fail some tests after osmlab/atlas#369

mgcuthbert
mgcuthbert previously approved these changes Mar 8, 2019
Copy link
Collaborator

@mgcuthbert mgcuthbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. Although I guess would need to look into what SonarQube is complaining about.

@@ -131,7 +131,9 @@

<!-- Miscellaneous other checks. -->
<module name="ArrayTypeStyle" />
<module name="AvoidEscapedUnicodeCharacters"/>
<module name="AvoidEscapedUnicodeCharacters">
<property name="allowEscapesForControlCharacters" value="true"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice I like this addition.

jklamer
jklamer previously approved these changes Mar 8, 2019
Copy link
Collaborator

@jklamer jklamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all this work

@matthieun
Copy link
Contributor Author

@mgcuthbert Sonar is complaining about this deprecated method:
#132 (comment)
If you have a better idea how to resolve that problem let me know!

@matthieun matthieun dismissed stale reviews from jklamer and mgcuthbert via 04ae757 March 8, 2019 18:35
yiqingj
yiqingj previously approved these changes Mar 8, 2019
Copy link
Collaborator

@yiqingj yiqingj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks like it's mostly just lib version upgrade and code formatting change.

I would recommend jumping major version number after merging this.

@yiqingj yiqingj merged commit 36240ee into osmlab:dev Mar 8, 2019
@matthieun matthieun deleted the jdk11 branch March 8, 2019 18:59
seancoulter pushed a commit to seancoulter/atlas-checks that referenced this pull request Dec 12, 2019
* update atlas & atlas-generator dependencies version

* continue to use atlas 5.1.13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants