-
Notifications
You must be signed in to change notification settings - Fork 59
Gradle Migration #166
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
Gradle Migration #166
Conversation
1. Migrated from Maven to Gradle. 2. Migrated workflows from Maven to Gradle. 3. Added Gradle related files to the .gitignore. 4. Added a Gradle plugin to handle license headers automatically (both `ConnectConfigTest.java` files and `DatabaseLifecycleTest.java` were missing headers, this makes sure that never happens again and keeps all headers consistent). See `HEADER`. 5. Suppressed all PMD and normal false positive warnings to make Gradle happy while fixing accurate warnings. 6. Used non deprecated alternatives to anything that was deprecated to make Gradle happy. 7. Added quoting to `detect_linux_distribution.sh` to prevent word splitting which would break the script. 8. Added `.gitattributes` to normalize line endings and protect the `gradle-wrapper.jar`.
|
@tomix26 after a lot more work than I expected, I've successfully migrated this project from Maven to Gradle. Let me know if I missed anything. I think this should be good to merge. All dependency versions are also nicely managed in the |
The binaries will still be compiled for JDK 8 but the JDK that runs Gradle will be 17.
|
@tomix26 the binaries will still be compiled for JDK 8 but the JDK that runs Gradle will be 17 (or higher) to support the latest Gradle version and to support running its plugins. This doesn't effect compatibility at all, what's shipped will still be JDK 8 (as seen in the Feel free to give the workflows another run. There shouldn't be anymore problems now with the changes I made. |
|
First off, thank you for the pull request and all your work. I really appreciate it. Your code looks very high-quality. However, there are far more changes here than we originally agreed upon. This pull request should really only contain changes related to the Maven to Gradle migration. For the other changes, we need to create separate, ideally smaller, pull requests that will be easier to review and manage. When a thousand changes are applied at once, it's difficult to guarantee that nothing breaks. Also, since this project is a fork of another project, I want to avoid the following types of changes:
I'm also not a big fan of cluttering the code with too many SuppressWarnings annotations, especially for static analysis purposes. So if possible, please try to configure PMD to more closely align with the previous findings. And the most important topic is compatibility with older Java versions. As I mentioned at the very beginning, tests must run against Java 8 and above. We can't rely on Therefore, I'd suggest first verifying whether it's possible in newer Gradle versions to ensure test execution with older Java versions. This is absolutely crucial, without this capability, it doesn't make sense to migrate to Gradle. |
|
The minimum supported Java version for Gradle is currently Java 17. So if this is the case, then sadly migration won't be possible. I'll close the issue and PR accordingly. |
|
I'm sorry to hear that. Feel free to create pull requests for the remaining changes that aren't related to the Gradle migration. Just keep in mind that smaller, separate PRs are much easier to review. Thank you. |

ConnectConfigTest.javafiles andDatabaseLifecycleTest.javawere missing headers, this makes sure that never happens again and keeps all headers consistent). SeeHEADER.detect_linux_distribution.shto prevent word splitting which would break the script..gitattributesto normalize line endings and protect thegradle-wrapper.jar.Closes #164