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
Static code analyzer improvement #145
Static code analyzer improvement #145
Conversation
Try change static code analyzer to checkstyle Wire up new static code analyzer
b5687a9
to
c6d6090
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like much easier tool to handle, I feel we can stick to that finally :)
frontendUi/src/main/java/com/virtuslab/gitmachete/frontend/ui/GitMacheteGraphTableManager.java
Outdated
Show resolved
Hide resolved
frontendUi/src/main/java/com/virtuslab/gitmachete/frontend/ui/table/GraphTableModel.java
Outdated
Show resolved
Hide resolved
gitCoreJGit/src/main/java/com/virtuslab/gitcore/impl/jgit/GitCoreRepository.java
Outdated
Show resolved
Hide resolved
frontendUi/src/main/java/com/virtuslab/gitmachete/frontend/ui/GitMachetePanel.java
Outdated
Show resolved
Hide resolved
A general note... it would be nice if some description were provided (there is a dedicated "comment field" while you are opening a PR). It makes changes easier to understand and review. Issue description commits messages, pr title usually cannot cover everything themself. In this case, it may be worth to provide and to have a look at: |
@micpiotrowski would be nice to regenerate the base CI image after this is merged (quite a lot of yet-unknown artifacts need to be downloaded, and spotbugs deps won't be needed anymore) |
gitCoreJGit/src/main/java/com/virtuslab/gitcore/impl/jgit/GitCoreLocalBranch.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes needed, otherwise approving, good job 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gj & awesome pr description!
${projectRoot}/config/checkstyle/checkstyle.xml
Suppression file is in- now it was removed${projectRoot}/config/checkstyle/suppressions.xml
and is included incheckstyle.xml
AvoidStarImport
in checkstyleImportControl
https://checkstyle.sourceforge.io/version/4.4/availablechecks.htmlhttps://checkstyle.sourceforge.io/checks.html - link to actual version but we use strictly version8.30
so in future this link will be https://checkstyle.sourceforge.io/version/8.30/checks.htmlImporting
subfiles (DTD ENTITY): Support Multiple Configuration Files checkstyle/checkstyle#991 (comment)