james-project
james-project copied to clipboard
apache
JAMES-3271 Gradle checkstyle
Attempt to run the checkstyle with Gradle based on @ieugen Gradle migration PR #217
The checkstyle for some reason never been able to work on test-classes with the Maven plugin, it's definitely obvious with Gradle that it works, as I have to fix many checkstyle issues along the way (so it's positive). It's not finished yet, still errors to fix before having a green build...
I could not manage to use https://github.com/openrewrite/rewrite-gradle-plugin , just keep saying can't find the dependency... I'm at a loss. It's not too much of a problem for now, as this lib is nice as if I understood it can fix itself checkstyle issues? But that could be the object of an other PR. Here I'm just trying to have the checkstyle running with the gradle build first :)
Status: ./gradlew clean build -x test --scan -> https://scans.gradle.com/s/kqaqnq6sh6xyq
You should try to merge into the gradle branch so we see only that diff. We might have to disable the plugin and address the checkstyle issues later. If you post the error for the rewrite, I might be able to help. I will check it out next week. Please note that chekstyle is something we can have extra, after the migration. We should focus first on the migration IMO.
Now all the test classes have been fixed :)
https://scans.gradle.com/s/pkw4c2ldx4xhs
You should try to merge into the gradle branch so we see only that diff.
We have for habit and practice to always merge on master branch, and if we need a work not merged yet, we compress it in one commit with a [NO_REVIEW] tag and the number of the PR for easily finding the code
If we merge the checkstyle fixes we will get a lot of conflicts with the existing PR's. That will create a lot of work to merge and/or throw away work that contributors did - which is not nice.
If we merge the checkstyle fixes we will get a lot of conflicts with the existing PR's.
Anyway we will have these conflicts at one moment or another. I suggest that we merge this as fast as possible to not have also conflicts on this PR.
If we merge the checkstyle fixes we will get a lot of conflicts with the existing PR's.
Anyway we will have these conflicts at one moment or another. I suggest that we merge this as fast as possible to not have also conflicts on this PR.
We could merge what it is now. My recommendation is to STOP the checkstyle work going further. Focus on merging the existing PR's and then resume checkstyle work. In the meantime we might have a better integration via the gradle plugin - with rewrite-gradle-plugin .
If we merge the checkstyle fixes we will get a lot of conflicts with the existing PR's.
Anyway we will have these conflicts at one moment or another. I suggest that we merge this as fast as possible to not have also conflicts on this PR.
We could merge what it is now. My recommendation is to STOP the checkstyle work going further. Focus on merging the existing PR's and then resume checkstyle work. In the meantime we might have a better integration via the gradle plugin - with rewrite-gradle-plugin .
The basic work on checkstyle is pretty much finished. It compiles with test classes fixed. Any further work with rewrite-gradle-plugin should be done later in an other PR, on this I agree :)
We could merge what it is now. My recommendation is to STOP the checkstyle work going further.
Agree any part of this work related to gradle MUST be delayed.
However checkstyle fixes are orthogonal to dependency management, thus handling them now in a separate PR would be a nice move.
However checkstyle fixes are orthogonal to dependency management, thus handling them now in a separate PR would be a nice move.
https://github.com/apache/james-project/pull/228
So far the gradle migration do not make progress, for the past two years.
I will close related PRs to reflect this matter of fact.
Don't hesitate to re-open if you work on this.