-
Notifications
You must be signed in to change notification settings - Fork 42
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
chore(build): Modernise and clean up build #504
Conversation
matrei
commented
Jan 25, 2024
- Implement buildSrc to isolate build related code and deps
- Rename gradle plugin project dir to gradle-plugin
- Add gradle dir for version catalogs
- Implement version catalogs to handle versions
- Remove redundant gradle files from subprojects
- Move grailsCentralPublishing.gradle to gradle dir
- Use project.layout to reference files and dirs
- Make build more type-safe with IDE code completion
- Unify quote usage
- Clean up dependencies and tighten their scopes
- Use lazy configuration where possible
- Remove unused ext properties
- Create workarounds for grails-gradle-plugin exposing groovy
- Remove used resources and attribute to asciidoctor task
- Fix classpath issues with groovydoc task
- Change 'docs' task as there is nothing to copy
- Change jar manifest to reflect projects properties
- Remove redundant Application.groovy from views-markup
- Update example projects
- Add missing CompileStatic and make compatible with Groovy 3.0.21-SNAPSHOT
- Implement buildSrc to isolate build related code and deps - Rename gradle plugin project dir to gradle-plugin - Add gradle dir for version catalogs - Implement version catalogs to handle versions - Remove redundant gradle files from subprojects - Move grailsCentralPublishing.gradle to gradle dir - Use project.layout to reference files and dirs - Make build more type-safe with IDE code completion - Unify quote usage - Clean up dependencies and tighten their scopes - Use lazy configuration where possible - Remove unused ext properties - Create workarounds for grails-gradle-plugin exposing groovy - Remove used resources and attribute to asciidoctor task - Fix classpath issues with groovydoc task - Change 'docs' task as there is nothing to copy - Change jar manifest to reflect projects properties - Remove redundant Application.groovy from views-markup - Update example projects - Add missing CompileStatic and make compatible with Groovy 3.0.21-SNAPSHOT
The snapshot repositories had accidentally been placed outside the configuration of the PublishingExtension.
By using the ORG_GRADLE_PROJECT_** env vars build files can be cleaned up from checking at multiple places for the values.
With this change the Grails version is only declared once.
The groovy version was not resolved correctly previously. With this change it gets set as three different environment vars. CI_GROOVY_VERSION and GROOVY_VERSION as regular environment vars and with ORG_GRADLE_PROJECT_groovyVersion that sets the groovyVersion project property. This allows for some choices in how to handle this in the build script. This change also adds a section to the settings.gradle file to allow for overriding the groovy version in the version catalog with the GROOVY_VERSION env variable.
The Groovydoc Gradle task need to be run with the same groovy version as Gradle.
AdoptOpenJDK got moved to Eclipe Temurin and won't be updated anymore.
With this change there is no need to check both environment variables and project properties. Also move the sonatype repo info to a project property.
With this change the workflow will use the versions of groovy and gradle enterprise that are defined in the build rather than hard-coding them in the actions file.
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Set up JDK | ||
uses: actions/setup-java@v4 | ||
- uses: gradle/wrapper-validation-action@v2 |
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 think we don't need to use gradle/wrapper-validation-action with gradle/actions/setup-gradle
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 see. But isn't it still a good protective step to prevent someone from committing a bad/malicious gradle wrapper? I can remove it if you think it's superfluous.
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.
@puneetbehl @matrei
FWIW I believe gradle/actions/setup-gradle
may not be validating binaries yet. At least that's my understanding given these Gradle issues:
- Automatically perform wrapper validation check in
setup-gradle
gradle/actions#12 - Adding a new dedicated workflow for validation should be discouraged gradle/wrapper-validation-action#94
- Merge/call validation during setup-gradle gradle/wrapper-validation-action#173
So it might be a good idea to explicitly invoke gradle/wrapper-validation-action
while gradle/actions#12 is not completed.
with: | ||
distribution: 'adopt' | ||
distribution: temurin |
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.
Why are using this specific distribution?
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.
From: https://github.com/actions/setup-java
NOTE: AdoptOpenJDK got moved to Eclipse Temurin and won't be updated anymore. It is highly recommended to migrate workflows from adopt and adopt-openj9, to temurin and semeru respectively, to keep receiving software and security updates. See more details in the Good-bye AdoptOpenJDK post.
.github/workflows/gradle.yml
Outdated
arguments: | | ||
build | ||
-Dgeb.env=chromeHeadless | ||
|
||
- name: Publish Test Report |
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.
Please also remove the step to publish test report, as we can see that with published build scan.
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.
Yes, I wondered about that one, but kept it in not to remove anything that was perhaps used somewhere.
These can now be viewed in build scans.
No need to run this on multiple versions
Puneet has a new email
@matrei Fantastic work! I truly appreciate all your help. |