Skip to content

Conversation

@tsmock
Copy link
Contributor

@tsmock tsmock commented Jan 13, 2022

Description:

Update remaining dependencies

Major updates:

  • osmosis 0.44 -> 0.48.3, required function renames from release -> close, as osmosis updated to Java 8 support
  • checkstyle from 8.18 to 9.2.1 -- for this, I had to use the version of atlas built in this PR for checkstyle checks due to checkstyle incompatibilities. The checkstyle update also forced some code changes, due to new/fixed checks
  • Gradle from 6.9.x to 7.3.3. While this was done independently, the changes are significantly similar to those already present in atlas-generator

Potential Impact:

Downstream users will need to update their dependencies (specifically, checkstyle), which will most likely force code changes.

Unit Test Approach:

N/A

Test Results:

Timings from ./gradlew run in atlas-checks for NZL:

  • 43956s (original)
  • 48499s (original)
  • 47463s (updates)
  • 49334s (updates)

I'm uncertain what happened with the two original runs (10%+ difference), however, the second non-updated run was about the same as the two runs with updates. It could be a simple as different software running on the laptop used.


In doubt: Contributing Guidelines

@tsmock tsmock force-pushed the dependency-updates branch from ac0fdff to 31b52c3 Compare January 13, 2022 18:37
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.2% 81.2% Coverage
0.0% 0.0% Duplication

matthieun
matthieun previously approved these changes Feb 16, 2022
Copy link
Collaborator

@matthieun matthieun left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks for doing all that maintenance!
Just a question about the upload of archives to maven central...

slf4j: "org.apache.logging.log4j:log4j-slf4j-impl:${versions.log4j}",
],
opencsv: "net.sf.opencsv:opencsv:${versions.opencsv}",
google_truth: "com.google.truth:truth:${versions.google_truth}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can thank the testing for Checkstyle for that, as I noted in a comment, "Google Truth is needed for checkstyle tests (as of checkstyle 9.2.1)".

}

uploadArchives
publishing
Copy link
Collaborator

Choose a reason for hiding this comment

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

So I remember this upload step being very finicky, but that was years back. Have you had any chance to test this new approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Short answer: no.

Longer answer: the comments indicated that it was due to two problems, travis-ci/travis-ci#9555 and an issue with uploads to the Sonatype repo requiring the same IP. AFAIK, atlas isn't using Travis anymore (but I could definitely be wrong about this), so 9555 isn't an issue. And I don't believe that GitHub actions will rotate IP address per upload.

It should work, but I've never published to Sonatype. I want to say one of the other projects I have worked on did something similar, but I cannot remember which project at this point.

This only updates dependencies that did not require code rewriting.

Dependencies that require code rewriting include osmosis, which blocks
protobuf.

Signed-off-by: Taylor Smock <[email protected]>
BeanUtils 1.9.2 added the ability to prevent access to the `class` field. BeanUtils 1.9.4
prevented this by default. The specific test used the `class` field to
check for non-instantiable object handling.

Signed-off-by: Taylor Smock <[email protected]>
This required updating some classes, as one of the primary differences
is support for Java 8 AutoCloseable.

Signed-off-by: Taylor Smock <[email protected]>
Signed-off-by: Taylor Smock <[email protected]>
Also, some prework for Gradle 7.x.

Signed-off-by: Taylor Smock <[email protected]>
Signed-off-by: Taylor Smock <[email protected]>
Signed-off-by: Taylor Smock <[email protected]>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.2% 81.2% Coverage
0.0% 0.0% Duplication

@adahn6 adahn6 merged commit 870a7a1 into osmlab:dev Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants