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

fix pmd warnings, codemetrics-2.1.0, gradle-8.4 #5146

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

soloturn
Copy link
Contributor

@soloturn soloturn commented Oct 5, 2023

gradle-8.4 released, fully supports java-21. codemetrics-2.1.0 has the pmd rules inlined, so warnings are gone now. see MovingBlocks/TeraConfig#19 . remove a couple of outdated TODOs. adjust a link in a comment to new gradle versions.

@soloturn soloturn force-pushed the develop branch 2 times, most recently from 1185e80 to e02cb3f Compare October 5, 2023 15:59
Comment on lines 24 to 25
// TODO MYSTERY: As of November 7th 2021 virtual-repo-live could no longer be relied on for latest snapshots - Pro feature?
// We've been using it that way for *years* and nothing likewise changed in the area for years as well. This seems to work ....
Copy link
Contributor

Choose a reason for hiding this comment

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

@Cervator - Do you know if this comment is still relevant with the new artifactory?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't tested but we should try doing so. With a little luck we can undo the hack. It was a huge upgrade over multiple major versions so it could well have changed the quirky bits a lot.

Copy link
Contributor Author

@soloturn soloturn Oct 6, 2023

Choose a reason for hiding this comment

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

if virtual repos do not work it is an artifactory bug. see feature matrix:
https://jfrog.com/help/r/jfrog-hosting-models-documentation/artifactory-comparison-matrix

where i fail to understand your reasoning: why do you want to direct all artifacts through your local repository, instead of listing a standard repo from internet + your own?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

took out the TODO removal, as controversal at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

So the artifacts specific to this issue actually is snapshot-level builds of our project artifacts - game modules, libraries, etc. They wouldn't be available on the regular internet repos :-)

I think Maven Central is in there somewhere to resolve 3rd party libs. Artifactory may also cache those - cannot quite recall the details.

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 are of course right, the standard repos are before. what distracted me was that the url you list:
https://artifactory.terasology.io/ui/native/virtual-repo-live

takes a while to load and lists internet as well. while i d have expected an url with terasoloty only.

Cervator
Cervator previously approved these changes Oct 8, 2023
Copy link
Member

@Cervator Cervator left a comment

Choose a reason for hiding this comment

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

Haven't tested specifically, but the changes look pretty trivial and reasonable 👍 Appreciate the doc update in particular!

@Cervator
Copy link
Member

Cervator commented Oct 8, 2023

Oh, and perhaps important since this talks about PMD: warnings during those scans were adding so much build time in Jenkins that I excluded PMD steps within the Jenkinsfile via f907533 (although huh, the PMD line didn't show up as part of the main commit line) - if this fixes that and restores build times to a decent range I'd be happy to see it hooked back up.

@soloturn
Copy link
Contributor Author

soloturn commented Oct 8, 2023

Oh, and perhaps important since this talks about PMD: warnings during those scans were adding so much build time in Jenkins that I excluded PMD steps within the Jenkinsfile via f907533 (although huh, the PMD line didn't show up as part of the main commit line) - if this fixes that and restores build times to a decent range I'd be happy to see it hooked back up.

there is a ticket saying logging got changed to finest which makes it 3 times slower: pmd/pmd#1018 . but for me it does not make a huge difference, local. so i updated the pmd version as well now to 6.55.0, lastest before 7.0.0, just to be sure no known bugs are in. if you want, release MovingBlocks/TeraConfig#22 this and i'll update codemetrics to it.

@soloturn soloturn changed the title fix pmd warnings, codemetrics-2.0.0, gradle-8.4 fix pmd warnings, codemetrics-2.1.0, gradle-8.4 Oct 15, 2023
@soloturn
Copy link
Contributor Author

this one could be merged now. updated codemetrics, so pmd warnings are gone as MovingBlocks/TeraConfig#22 is used, thanks @jdrueckert !

gradle-8.4 fully supports java-21, update link in comment because of
new gradle version.

codemetrics-2.1.0, has the pmd rules inlined, so warnings are gone
now. see MovingBlocks/TeraConfig#19 and MovingBlocks/TeraConfig#22.

update pmd to 6.55.0, last release before 7.0.0.

update spotbugs gradle plugin to 5.2.0 to makeit work with java-21.
Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

LGTM

@BenjaminAmos BenjaminAmos merged commit 1494c02 into MovingBlocks:develop Oct 16, 2023
8 checks passed
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.

4 participants