jdaugherty commented on code in PR #16025:
URL: https://github.com/apache/grails-core/pull/16025#discussion_r3670757375
##########
.github/workflows/release.yml:
##########
@@ -690,7 +698,7 @@ jobs:
# Runs on the JDK 21 pin, where settings.gradle auto-prunes the
Micronaut island.
run: ./gradlew grails-doc:build -PgithubBranch=${TARGET_BRANCH}
- name: "🚀 Publish to GitHub Pages"
- uses: apache/grails-github-actions/deploy-github-pages@asf
+ uses:
apache/grails-github-actions/deploy-github-pages@fe90f651c47c5230cdd70b6fee0e0d250fea30ac
# asf
Review Comment:
We should not be pinning our own actions.
##########
.github/workflows/vulnerability-scan.yml:
##########
@@ -36,9 +36,9 @@ jobs:
contents: read
steps:
- name: "📥 Checkout repository"
- uses: actions/checkout@v6
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #
v6.0.2
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/release.yml:
##########
@@ -87,7 +87,7 @@ jobs:
cache-provider: basic # 'basic' uses the MIT-licensed, open-source
cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle
commercial Terms of Use)
develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
- name: "⚙️ Run pre-release"
Review Comment:
We should never pin our own actions
##########
.github/workflows/release.yml:
##########
@@ -247,7 +247,7 @@ jobs:
# downstream checksum/artifact-list combination steps all expect the
# default JDK 21 toolchain. Also keeps any future steps that touch the
# repository's own (non-Micronaut) Gradle config on the documented JDK.
- uses: actions/setup-java@v4
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 #
v5.2.0
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -694,7 +694,7 @@ jobs:
distribution: liberica
java-version: 21
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -950,7 +950,7 @@ jobs:
compression-level: 0 # Already a zip
- name: "🚀 Publish to Github Pages"
if: github.event_name == 'push'
- uses: apache/grails-github-actions/deploy-github-pages@asf
+ uses:
apache/grails-github-actions/deploy-github-pages@fe90f651c47c5230cdd70b6fee0e0d250fea30ac
# asf
Review Comment:
We should not pin our own actions
##########
.github/workflows/release.yml:
##########
@@ -373,11 +374,18 @@ jobs:
find . -depth \( -type f -o -type d \) -exec touch -d
"@${SOURCE_DATE_EPOCH}" {} +
- name: "📦 Create source distribution ZIP"
run: >
- zip -r
+ zip -yr
${DIST_NAME}-${VERSION}-src.zip
grails
-x 'grails/.git/*'
-x 'grails/.github/*'
+ -x 'grails/.omo/*'
Review Comment:
These should be gitignored, or we should discuss as a team on when to check
these in.
##########
.github/workflows/gradle.yml:
##########
@@ -923,7 +923,7 @@ jobs:
distribution: liberica
java-version: 21
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -81,7 +81,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy:
##########
@@ -161,11 +163,12 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
// Redirect XML report output to a single directory to consolidate
// reports across all subprojects into one known location
task.reports.xml.outputLocation.set(
- project.extensions.getByType(GrailsCodeStyleExtension)
- .reportsDirectory.get()
- .dir('checkstyle')
- .file("${project.name}-${task.name}.xml")
+
project.extensions.getByType(GrailsCodeStyleExtension)
Review Comment:
The formatting has been messed up and needs fixed.
##########
.github/workflows/vulnerability-scan.yml:
##########
@@ -36,9 +36,9 @@ jobs:
contents: read
steps:
- name: "📥 Checkout repository"
- uses: actions/checkout@v6
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #
v6.0.2
- name: "☕️ Setup JDK"
- uses: actions/setup-java@v4
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 #
v5.2.0
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -143,7 +143,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/release.yml:
##########
@@ -224,7 +224,7 @@ jobs:
# staged from a JDK 25 runner. This is a NEW reproducibility pin -
# keep $JAVA_VERSION_MICRONAUT synced with the secondary JDK in
# etc/bin/Dockerfile so verifiers can reproduce the resulting JARs.
- uses: actions/setup-java@v4
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 #
v5.2.0
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
gradle/rat-root-config.gradle:
##########
@@ -20,6 +20,9 @@ apply plugin: 'org.nosphere.apache.rat'
tasks.named('rat') {
def allExcludes = [
'.asf.yaml', // ASF metadata for github integration excluded from
src zip
+ '.omo/**', // local agent work state excluded from src zip
+ '.claude/**', '.clinerules', '.cursorrules', '.windsurfrules', //
local agent configuration excluded from src zip
Review Comment:
We need to update the .gitignore file to exclude these as well or make sure
our release process explicitly removes them. I don't see entries for a lot of
these.
##########
.github/workflows/release-publish-docs.yml:
##########
@@ -74,7 +74,7 @@ jobs:
# The docs themselves have no code dependency on Micronaut.
run: ./gradlew grails-doc:build -PgithubBranch=${TARGET_BRANCH}
- name: "🚀 Publish to GitHub Pages"
- uses: apache/grails-github-actions/deploy-github-pages@asf
+ uses:
apache/grails-github-actions/deploy-github-pages@fe90f651c47c5230cdd70b6fee0e0d250fea30ac
# asf
Review Comment:
we should never pin our own actions
##########
.github/workflows/release.yml:
##########
@@ -765,7 +773,7 @@ jobs:
ref: ${{ env.TAG }}
token: ${{ secrets.GITHUB_TOKEN }}
- name: "⚙️ Run post-release"
- uses: apache/grails-github-actions/post-release@asf
+ uses:
apache/grails-github-actions/post-release@fe90f651c47c5230cdd70b6fee0e0d250fea30ac
# asf
Review Comment:
we should not be pinning our own actions
##########
gradle.properties:
##########
@@ -72,6 +72,9 @@ pmdVersion=7.25.0
spotbugsPluginVersion=6.4.8
sonatypeScanPluginVersion=3.1.6
+# PMD is enforced only for projects with a clean baseline. Add project paths
after clearing debt.
+grails.code-analysis.enabled.pmd.projects=:grails-data-graphql-core,:grails-data-mongodb-spring-data,:grails-datasource,:grails-testing-support-core
Review Comment:
I don't think we should be centrally setting this. Before it was opt in on
a project base. this list is going to grow unreasonable large and it's hard
coding project names which make this fragile.
##########
.github/workflows/release.yml:
##########
@@ -373,11 +374,18 @@ jobs:
find . -depth \( -type f -o -type d \) -exec touch -d
"@${SOURCE_DATE_EPOCH}" {} +
- name: "📦 Create source distribution ZIP"
run: >
- zip -r
+ zip -yr
${DIST_NAME}-${VERSION}-src.zip
grails
-x 'grails/.git/*'
-x 'grails/.github/*'
+ -x 'grails/.omo/*'
+ -x 'grails/.claude/*'
+ -x 'grails/.clinerules'
+ -x 'grails/.cursorrules'
+ -x 'grails/.windsurfrules'
+ -x 'grails/CLAUDE.md'
Review Comment:
CLAUDE.md should be in the source zip - we're using it to develop so it
shouldn't be excluded.
##########
.github/workflows/groovy-joint-workflow.yml:
##########
@@ -105,7 +105,7 @@ jobs:
runs-on: ubuntu-latest
services:
mongodb:
- image: mongo:8
+ image:
mongo@sha256:951c2ff9fc6bdb6cb89b1dfea4a0e8ae3ee4fb287c0bf579b2bba54c7803f75d #
mongo:8
Review Comment:
Have we configured dependabot to auto-suggest these updates? I agree with
pinning this one though.
##########
.github/workflows/release-close.yml:
##########
@@ -45,7 +45,7 @@ jobs:
ref: ${{ env.TAG }}
token: ${{ secrets.GITHUB_TOKEN }}
- name: "⚙️ Run post-release"
- uses: apache/grails-github-actions/post-release@asf
+ uses:
apache/grails-github-actions/post-release@fe90f651c47c5230cdd70b6fee0e0d250fea30ac
# asf
Review Comment:
we should never pin our own actions
##########
.github/workflows/gradle.yml:
##########
@@ -628,7 +628,7 @@ jobs:
distribution: liberica
java-version: 21
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -812,7 +812,7 @@ jobs:
distribution: liberica
java-version: 21
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -193,7 +193,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -323,7 +323,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -495,7 +495,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeAnalysisPlugin.groovy:
##########
@@ -170,16 +178,25 @@ class GrailsCodeAnalysisPlugin implements Plugin<Project>
{
def xmlReport = spotBugsReports.maybeCreate('xml')
xmlReport.required.set(true)
xmlReport.outputLocation.set(
- project.extensions.getByType(GrailsCodeAnalysisExtension)
- .reportsDirectory.get()
- .dir('spotbugs')
- .file("${project.name}-${it.name}.xml")
+
project.extensions.getByType(GrailsCodeAnalysisExtension)
Review Comment:
The formatting here has been messed up and should be restored.
##########
.github/workflows/gradle.yml:
##########
@@ -547,7 +547,7 @@ jobs:
distribution: liberica
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/codeanalysis.yml:
##########
@@ -95,3 +113,22 @@ jobs:
file="grails-gradle/build/reports/violations/$report"
[ -f "$file" ] && cat "$file" >> $GITHUB_STEP_SUMMARY || true
done
+ - name: "🔎 Run Gradle Plugin SpotBugs Advisory"
+ if: always()
+ continue-on-error: true
+ working-directory: grails-gradle
+ run: |
+ rm -f build/reports/violations/SPOTBUGS_VIOLATIONS.md
+ ./gradlew aggregateAnalysisViolations --continue
-Pgrails.code-analysis.enabled.spotbugs=true
-Pgrails.code-analysis.ignoreFailures=true
+ - name: "📤 Upload Gradle Plugin SpotBugs Advisory Reports"
+ if: always()
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -74,6 +74,8 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
static final String DEFAULT_JACOCO_EXCLUDED_CLASS_PREFIXES =
'org.grails.orm.hibernate.support.hibernate7.'
+ private static final String CANONICAL_ROOT_MARKER = '.github/workflows'
Review Comment:
We already have a utility in GradleUtil to find the root directory. We
should be consistent and use that instead.
##########
.github/workflows/gradle.yml:
##########
@@ -242,7 +242,7 @@ jobs:
distribution: 'liberica'
java-version: ${{ matrix.java }}
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/gradle.yml:
##########
@@ -37,7 +37,7 @@ jobs:
distribution: liberica
java-version: 21
- name: "🗄️ Restore dependency jar cache"
- uses: actions/cache@v4
+ uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
.github/workflows/codeanalysis.yml:
##########
@@ -60,6 +60,24 @@ jobs:
file="build/reports/violations/$report"
[ -f "$file" ] && cat "$file" >> $GITHUB_STEP_SUMMARY || true
done
+ - name: "🔎 Run Core SpotBugs Advisory"
+ if: always()
+ continue-on-error: true
+ run: |
+ rm -f build/reports/violations/SPOTBUGS_VIOLATIONS.md
+ ./gradlew aggregateAnalysisViolations --continue
"-Pgrails.code-analysis.enabled.pmd.projects="
-Pgrails.code-analysis.enabled.spotbugs=true
-Pgrails.code-analysis.ignoreFailures=true
+ - name: "📤 Upload SpotBugs Advisory Reports"
+ if: always()
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
Review Comment:
for actions, we had been specifying the non-commit version so they pull the
latest. This aligned with the ASF approvals upstream.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy:
##########
@@ -161,11 +163,12 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
// Redirect XML report output to a single directory to consolidate
// reports across all subprojects into one known location
task.reports.xml.outputLocation.set(
- project.extensions.getByType(GrailsCodeStyleExtension)
- .reportsDirectory.get()
- .dir('checkstyle')
- .file("${project.name}-${task.name}.xml")
+
project.extensions.getByType(GrailsCodeStyleExtension)
+ .reportsDirectory
+
.file("checkstyle/${GradleUtils.reportFileName(project, task.name)}")
Review Comment:
We shoudl restore the .dir().file() like before
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy:
##########
@@ -201,11 +205,12 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
// reports across all subprojects into one known location
task.reports.xml.required.set(true)
task.reports.xml.outputLocation.set(
- project.extensions.getByType(GrailsCodeStyleExtension)
- .reportsDirectory.get()
- .dir('codenarc')
- .file("${project.name}-${task.name}.xml")
+
project.extensions.getByType(GrailsCodeStyleExtension)
Review Comment:
Formatting needs restored.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy:
##########
@@ -201,11 +205,12 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
// reports across all subprojects into one known location
task.reports.xml.required.set(true)
task.reports.xml.outputLocation.set(
- project.extensions.getByType(GrailsCodeStyleExtension)
- .reportsDirectory.get()
- .dir('codenarc')
- .file("${project.name}-${task.name}.xml")
+
project.extensions.getByType(GrailsCodeStyleExtension)
+ .reportsDirectory
+
.file("codenarc/${GradleUtils.reportFileName(project, task.name)}")
Review Comment:
.dir().file()
##########
build-logic/plugins/build.gradle:
##########
@@ -40,6 +40,7 @@ dependencies {
implementation
"org.cyclonedx.bom:org.cyclonedx.bom.gradle.plugin:${gradleProperties.gradleCycloneDxPluginVersion}"
implementation
"com.github.spotbugs.snom:spotbugs-gradle-plugin:${gradleProperties.spotbugsPluginVersion}"
implementation
"org.sonatype.gradle.plugins:scan-gradle-plugin:${gradleProperties.sonatypeScanPluginVersion}"
+ implementation 'org.yaml:snakeyaml:2.4'
Review Comment:
We should be pulling the version from our dependencies or set it in our bom.
2.4 mismatches the upstream bom version (2.6)
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeAnalysisPlugin.groovy:
##########
@@ -170,16 +178,25 @@ class GrailsCodeAnalysisPlugin implements Plugin<Project>
{
def xmlReport = spotBugsReports.maybeCreate('xml')
xmlReport.required.set(true)
xmlReport.outputLocation.set(
- project.extensions.getByType(GrailsCodeAnalysisExtension)
- .reportsDirectory.get()
- .dir('spotbugs')
- .file("${project.name}-${it.name}.xml")
+
project.extensions.getByType(GrailsCodeAnalysisExtension)
+ .reportsDirectory
+ .file("spotbugs/${GradleUtils.reportFileName(project,
it.name)}")
Review Comment:
We should restore the original format .dir().file() instead of using a
gstring
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
