-
Notifications
You must be signed in to change notification settings - Fork 126
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
Update PMD to 7.0.0 #689
Update PMD to 7.0.0 #689
Conversation
AGP 8.0 upgrade assistant transforms those to build.gradle instructions which aren't supported for pure java libs
build.gradle
Outdated
@@ -7,7 +7,7 @@ buildscript { | |||
google() | |||
} | |||
dependencies { | |||
classpath 'com.android.tools.build:gradle:7.4.2' | |||
classpath 'com.android.tools.build:gradle:8.3.1' |
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.
Note that many developers use IDEA instead of Android Studio.
Latest IDEA does not still support the Android Gradle plugin 8.3.X.
IDEA works fine with Android Gradle plugin 8.2.X (currently 8.2.2
).
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.
I guess we don't use any features from 8.3.x, so switching back to 8.2.2 shoudn't cause any issues.
@@ -16,10 +16,10 @@ jobs: | |||
environment: BRouter | |||
steps: | |||
- uses: actions/checkout@v4 | |||
- name: Set up JDK 11 | |||
- name: Set up JDK 17 |
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.
One remark only, the BRouter server still uses JDK 11.
Therefore, the output lib cannot be used directly.
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.
If we want to compile for older Java releases we should target a specific java version. It seems reasonable to target Java 11 but use JDK 17 to build.
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.
For maximum compatibility with old Android versions (that do not contain newer Java API),
in the open source libraries I maintain, we may use Java 17 to compile,
but have sourceCompatibility
and targetCompatibility
with JavaVersion.VERSION_1_8
.
For example:
https://github.com/mapsforge/mapsforge/blob/master/build.gradle#L32-L33
https://github.com/mapsforge/vtm/blob/master/build.gradle#L34-L35
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.
Fine, I could try sourceCompatibility and targetCompatibility next week with the test server.
gradle userguide suggests to avoid allprojects/subprojects and use conventions instead https://docs.gradle.org/current/userguide/sharing_build_logic_between_subprojects.html#sec:convention_plugins_vs_cross_configuration
I've implemented targeting a specific Java version using conventions plugins because the gradle documentation suggested to avoid cross configuration using allprojects/subprojects Currently it targets Java 11 because we already use some functionality from Java 9. It's mostly in test code but the new Hgt processing also uses it. This code won't be run on Android so it should not cause any issues. Going forward we should pick a Java version we really want to target and adopt the build configuration to treat using newer code as error. I'd also like to add a dependency from |
I made some tests:
|
As you see in the current release, the distribution file is not produced.
|
Your suggestion would break builds without android SDK because the |
Yes, you are right. This is only a temporary solution to get an apk inside the zip when Android tasks are present. |
I've masked the dependencies now, that should do the job.
|
I'm not to happy with this check because not we check for |
@zod |
PMD has finally released version 7.0.0 which offers better analysis and new rules.
It required a gradle update but Android Studio kept nagging me to upgrade gradle & AGP (Android Gradle Plugin) anyway...