Skip to content

Conversation

@Pankraz76
Copy link
Contributor

@Pankraz76 Pankraz76 commented May 28, 2025

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MNG-XXX] SUMMARY,
    where you replace MNG-XXX and SUMMARY with the appropriate JIRA issue.
  • Also format the first line of the commit message like [MNG-XXX] SUMMARY.
    Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 32c2936 to 185d432 Compare May 28, 2025 17:16
@Pankraz76 Pankraz76 changed the title PoC rule: category/java/bestpractices.xml/MissingOverride [PoC] rule: category/java/bestpractices.xml/MissingOverride May 28, 2025
@Pankraz76
Copy link
Contributor Author

Pankraz76 commented May 29, 2025

tried to integrate your feedback @slachiewicz, is this any better?

@Pankraz76 Pankraz76 changed the title [PoC] rule: category/java/bestpractices.xml/MissingOverride [S1161] rule: category/java/bestpractices.xml/MissingOverride Jun 3, 2025
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 185d432 to 382223a Compare June 3, 2025 20:38
@slachiewicz
Copy link
Member

ruleset.xml should have Apache license header

@Pankraz76
Copy link
Contributor Author

yes of course, thanks.

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 382223a to 75eea01 Compare June 5, 2025 06:40
@Pankraz76 Pankraz76 marked this pull request as ready for review June 5, 2025 06:40
Pankraz76

This comment was marked as off-topic.

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 617fa5c to 36cef6e Compare June 5, 2025 18:53
Copy link
Member

@slachiewicz slachiewicz left a comment

Choose a reason for hiding this comment

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

Not urgent, nice to have. To me it's a manual operation to fix.
As recommended earlier, not worth trying to touch deprecated code/module.

@slachiewicz
Copy link
Member

Small comment, issue template contains some statements about license and copyright. Please restore the template and agree to terms

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 36cef6e to 744300f Compare June 5, 2025 19:58
@Pankraz76
Copy link
Contributor Author

Pankraz76 commented Jun 5, 2025

fixed <exclude-pattern>.*/compat/maven-compat/.*</exclude-pattern>.

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch 2 times, most recently from 493346d to 1673c08 Compare June 6, 2025 16:14
@Pankraz76
Copy link
Contributor Author

Pankraz76 commented Jun 6, 2025

now the math adds up:

141 files in fixes:

plus

  • 3 files remaining here

will finally fix this.

add up 144 files in this PR.

@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 1673c08 to f00785b Compare June 6, 2025 16:50
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch 2 times, most recently from 043ab34 to 3647be1 Compare June 7, 2025 15:49
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch 2 times, most recently from 23b62c2 to 3f330dc Compare June 7, 2025 15:57
@Pankraz76 Pankraz76 requested a review from elharo June 7, 2025 15:58
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 3f330dc to 2dbe9a8 Compare June 8, 2025 19:46
@Pankraz76 Pankraz76 requested a review from elharo June 8, 2025 19:46
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from 2dbe9a8 to 6493deb Compare June 9, 2025 09:14
@Pankraz76 Pankraz76 requested a review from adangel June 9, 2025 09:14
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Maybe the title should be just "Enable PMD check MissingOverride"?

@Pankraz76 Pankraz76 changed the title [S1161] rule: category/java/bestpractices.xml/MissingOverride [S1161] Enable PMD check MissingOverride Jun 9, 2025
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch 2 times, most recently from 5a1fe5a to a7d1632 Compare June 9, 2025 11:11
Copy link
Contributor Author

@Pankraz76 Pankraz76 left a comment

Choose a reason for hiding this comment

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

whats good default value for analysisCacheLocation?

pom.xml Outdated
Comment on lines 802 to 803
<analysisCache>true</analysisCache>
<analysisCacheLocation>${maven.multiModuleProjectDirectory}/.pmd/.cache</analysisCacheLocation>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
<analysisCache>true</analysisCache>
<analysisCacheLocation>${maven.multiModuleProjectDirectory}/.pmd/.cache</analysisCacheLocation>
<analysisCache>true</analysisCache>
<analysisCacheLocation>${maven.multiModuleProjectDirectory}/.pmd/.cache</analysisCacheLocation>

would give here CoC value as well, to avoid confusion.
is central cache good or better dedicated to each module.

Having both variants implied, so one way is wrong already, as done different, assuming this valid as not mentioned by you @adangel.

Giving nifty default value to analysisCacheLocation wont hurt, right?

Copy link
Member

Choose a reason for hiding this comment

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

The defaults are documented in the documentation (surprise!) https://maven.apache.org/plugins/maven-pmd-plugin/pmd-mojo.html#analysisCache

Not sure, what "maven.multiModuleProjectDirectory" would resolve to - probably to the same directory for every single module. Hence this config probably results in overwriting the cache file on every run - and the cache is useless.

Did you try whether the cache is working? Running "./mvnw clean verify" and again "./mvnw verify" (without clean) and comparing the timings of the pmd plugin?

The default value seems to be more sensible, as it uses a cache file per module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ty.

Copy link
Contributor Author

@Pankraz76 Pankraz76 Jun 9, 2025

Choose a reason for hiding this comment

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

2 sec.

image

clean.

init takes 4 sec.

image

switch working

50% gain:

image

@Pankraz76 Pankraz76 requested a review from adangel June 9, 2025 11:18
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from a7d1632 to bc4b52c Compare June 9, 2025 20:44
@Pankraz76 Pankraz76 force-pushed the MissingOverride-pmd branch from bc4b52c to d019b9c Compare June 9, 2025 20:51
@slachiewicz slachiewicz closed this Jun 9, 2025
@Pankraz76
Copy link
Contributor Author

striving in chaos.

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