Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Add PMD for static analysis #102

Merged
merged 12 commits into from
Aug 9, 2020
Merged

Add PMD for static analysis #102

merged 12 commits into from
Aug 9, 2020

Conversation

sladyn98
Copy link
Contributor

@sladyn98 sladyn98 commented Aug 1, 2020

No description provided.

@sladyn98 sladyn98 added the chore label Aug 1, 2020
@sladyn98 sladyn98 added this to the GSoC- Phase - 3 milestone Aug 1, 2020
@sladyn98 sladyn98 requested a review from a team as a code owner August 1, 2020 05:41
@sladyn98 sladyn98 linked an issue Aug 1, 2020 that may be closed by this pull request
Add FindBugs and PMD #97
Closed
@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 1, 2020

This build fails because PMD throws errors, should I fix the errors in this PR itself ?

martinda
martinda suggested changes Aug 1, 2020
View reviewed changes
Copy link
Contributor

@martinda martinda left a comment

Choose a reason for hiding this comment

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

Yes you need to fix the errors in the same PR. You may also need a pmd xml configuration file for the pmd rules.

@sladyn98 sladyn98 requested a review from martinda August 3, 2020 08:13
@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 3, 2020

@martinda These were thrown by findbugs so I fixed them else it was failing? Should I revert the commit and just let the pmd.xml remain ?

martinda
martinda suggested changes Aug 3, 2020
View reviewed changes
Copy link
Contributor

@martinda martinda left a comment

Choose a reason for hiding this comment

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

See review comments.

config/pmd.xml Outdated Show resolved Hide resolved
config/pmd.xml Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@martinda
Copy link
Contributor

martinda commented Aug 3, 2020

@martinda These were thrown by findbugs so I fixed them else it was failing? Should I revert the commit and just let the pmd.xml remain ?

To make things easy and clear, please present two pull-requests. One for PMD, one for findbugs. Please do not lose my comments on PMD. So yes it is probably best to move findbugs to another PR and do just PMD in this one.

kwhetstone
kwhetstone reviewed Aug 4, 2020
View reviewed changes
Copy link
Contributor

@kwhetstone kwhetstone left a comment

Choose a reason for hiding this comment

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

If the findbugs causes the build to fail, then you would need to fix them here. Otherwise, just keep this fixing the PMD and we'll be able to merge in the other pieces.

config/pmd.xml Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
src/main/java/com/org/jenkins/custom/jenkins/distribution/service/generators/WarGenerator.java Outdated Show resolved Hide resolved
@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 4, 2020

@kwhetstone @martinda Yeah the findbugs is causing the build to fail hence I fixed them here.

@sladyn98 sladyn98 changed the title Add findbugs and PMD for static analysis Add PMD for static analysis Aug 4, 2020
martinda
martinda suggested changes Aug 4, 2020
View reviewed changes
Copy link
Contributor

@martinda martinda left a comment

Choose a reason for hiding this comment

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

Please address these changes, they enhance the clarity.

config/pmd.xml Show resolved Hide resolved
...n/java/com/org/jenkins/custom/jenkins/distribution/service/services/UpdateCenterService.java Outdated Show resolved Hide resolved
@kwhetstone
Copy link
Contributor

You have 58 PMD violations. For more details see: C:\Jenkins\workspace\stom-distribution-service_PR-102\target\pmd.xml ?

@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 6, 2020

@kwhetstone yeah working on fixing as much as I can slowly , cuz some of them are a real pain

@martinda martinda merged commit 1f1935b into jenkinsci:master Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kwhetstone kwhetstone kwhetstone left review comments

@martinda martinda martinda approved these changes

Assignees
No one assigned
Labels
chore
Projects
None yet
Milestone
GSoC- Phase - 3
Development

Successfully merging this pull request may close these issues.

Add FindBugs and PMD
3 participants
@sladyn98 @martinda @kwhetstone