jamesfredley commented on code in PR #16025:
URL: https://github.com/apache/grails-core/pull/16025#discussion_r3677926230
##########
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:
Addressed in `7a0aceb5ff` with restored layout/report path construction and
formatting cleanups in the aggregation/analysis plugins.
##########
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:
Addressed in `7a0aceb5ff` with restored layout/report path construction and
formatting cleanups in the aggregation/analysis plugins.
##########
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:
Addressed in `7a0aceb5ff` with restored layout/report path construction and
formatting cleanups in the aggregation/analysis plugins.
##########
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:
Addressed in `7a0aceb5ff` with restored layout/report path construction and
formatting cleanups in the aggregation/analysis plugins.
##########
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:
Addressed in `7a0aceb5ff`. Canonical-root detection uses
`GradleUtils.findRootGrailsCoreDir` / `.asf.yaml` consistently. `9a8fbeed87`
also keeps `.asf.yaml` in the source ZIP and terminates root search at the
filesystem root.
##########
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:
Addressed in `7a0aceb5ff`. SnakeYAML is managed via `snakeyamlVersion` (2.6)
rather than a hardcoded 2.4 literal.
##########
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:
Addressed in `7a0aceb5ff`. Central `gradle.properties` allowlist removed;
modules opt in with `grailsCodeAnalysis { pmdEnabled = true }`. Global `-P`
overrides remain.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
+ include '**/messages*.properties'
+ exclude '**/build/**', '**/generated/**'
+ }
+ )
+ task.reportFile.set(violationsDir.map {
it.file('REPOSITORY_CONVENTIONS.md') })
+ task.outputs.upToDateWhen { false }
+ task.dependsOn(root.tasks.matching { Task candidate ->
candidate.name == 'rat' })
}
}
- private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> styleXmlDir, Provider<Directory> violationsDir) {
- // Wire property flags as Providers — values are resolved at task
execution time, not at apply() time,
- // and Providers are configuration-cache safe to capture in task
actions
+ private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> violationsDir) {
+ Directory rootDirectory = root.layout.projectDirectory
def checkStyleTests = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.TEST_STYLING_PROPERTY)
+ def ignoreFailures = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.IGNORE_FAILURES_PROPERTY)
def codenarcEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CODENARC_ENABLED_PROPERTY, true)
def checkstyleEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CHECKSTYLE_ENABLED_PROPERTY, true)
+ def codenarcMarkers = root.files()
+ def checkstyleMarkers = root.files()
+ def codenarcReports = root.files()
+ def checkstyleReports = root.files()
+ def codenarcMarkdown =
root.layout.buildDirectory.file('reports/violations/CODENARC_VIOLATIONS.md')
+ def checkstyleMarkdown =
root.layout.buildDirectory.file('reports/violations/CHECKSTYLE_VIOLATIONS.md')
+ def cleanupTask = root.tasks.register('cleanAggregateStyleReports') {
+ it.doLast {
+ deleteReports(codenarcMarkers.files, codenarcReports.files)
+ deleteReports(checkstyleMarkers.files, checkstyleReports.files)
+ }
+ }
- def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ def writerTask = root.tasks.register('writeStyleViolations') {
it.group = 'verification'
- it.description = 'Aggregates CodeNarc and Checkstyle violation
reports into build/reports/violations/'
-
it.outputs.file(root.file('build/reports/violations/CODENARC_VIOLATIONS.md'))
-
it.outputs.file(root.file('build/reports/violations/CHECKSTYLE_VIOLATIONS.md'))
+ it.description = 'Writes CodeNarc and Checkstyle violation reports
into build/reports/violations/'
+ it.inputs.files(codenarcMarkers).optional()
+ it.inputs.files(checkstyleMarkers).optional()
+ it.inputs.property('ignoreFailures', ignoreFailures)
+ it.outputs.file(codenarcMarkdown)
+ it.outputs.file(checkstyleMarkdown)
+ it.outputs.upToDateWhen { false }
+ it.doFirst {
+ codenarcMarkdown.get().asFile.delete()
+ checkstyleMarkdown.get().asFile.delete()
+ }
it.doLast {
- parseStyleViolations(styleXmlDir.get(), violationsDir.get(),
- checkStyleTests.get(), codenarcEnabled.get(),
checkstyleEnabled.get())
+ parseStyleViolations(codenarcMarkers.files,
checkstyleMarkers.files, rootDirectory, violationsDir.get(),
+ checkStyleTests.get(), codenarcEnabled.get(),
+ checkstyleEnabled.get(), ignoreFailures.get())
}
}
- root.subprojects { Project sub ->
- sub.pluginManager.withPlugin('codenarc') {
- aggregateTask.configure {
- it.dependsOn(sub.tasks.withType(CodeNarc))
- }
+ def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ it.group = 'verification'
+ it.description = 'Aggregates CodeNarc and Checkstyle violations
into build/reports/violations/'
+ it.dependsOn(writerTask)
+ }
+ if (root.tasks.names.contains('validateRepositoryConventions')) {
+ aggregateTask.configure {
it.dependsOn(root.tasks.named('validateRepositoryConventions')) }
+ }
+ def finalizeTask = root.tasks.register('finalizeStyleViolations') {
+ it.group = 'verification'
+ it.dependsOn(writerTask)
+ }
+ root.allprojects { Project sub ->
+ sub.tasks.withType(CodeNarc).all { CodeNarc codeNarcTask ->
Review Comment:
Addressed in `7a0aceb5ff`. Quality wiring uses `configureEach` and
collection-based `dependsOn`/`mustRunAfter` so ordinary tasks no longer realize
every analyzer.
--
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]
