The GitHub Actions job "CI" on grails-core.git/spike/grails-beans-dsl has 
failed.
Run started by GitHub user codeconsole (triggered by codeconsole).

Head commit for run:
79b2f3f15837ab08f11eca232f21f0459ef4ac05 / Scott Murphy Heiberg 
<[email protected]>
Address review findings: publishing, static compilation, arg validation, silent 
failures

An external agent review of this PR surfaced four findings. All four
are addressed here.

- grails-beans-dsl was never actually published: it applied neither
  buildsrc.publish nor buildsrc.sbom, and was absent from
  publishedProjects in gradle/publish-root-config.gradle, so the
  org.apache.grails:grails-beans-dsl coordinate the guide tells
  external users to depend on would not have existed. Registered the
  module and re-applied both plugins; verified a real POM now
  generates with the correct coordinates.

- The review flagged that @CompileStatic on a Plugin class was not
  propagated to the generated sibling AutoConfiguration. Attempted
  the fix (copying the annotation across), then verified via a real
  bytecode-level test (disassembling the compiled sibling and
  checking for invokedynamic) that copying the annotation achieves
  nothing: Groovy schedules static compilation by scanning for
  @CompileStatic before this transform's own callback runs, so a
  class created during that callback is never a candidate for it
  regardless of what it's annotated with. Reverted the non-functional
  copy - keeping it would have been actively misleading, claiming
  static compilation the bytecode doesn't have - and corrected the
  docs/javadoc to state the real, now-verified boundary: the
  standalone-class form gets genuine static dispatch, the
  Plugin-subclass sibling never does. Both are now pinned by
  bytecode-level regression tests.

- bean(...) argument validation only checked the first argument and
  read the second opportunistically: bean(String, someVariable) {}
  silently discarded the variable and fell back to a decapitalized
  type name, bean(String, 'x', 'unexpected') {} silently ignored the
  extra argument, and a non-String constant (e.g. bean(String, 42))
  would have been stringified into an invalid generated method name.
  Rewrote the validation to strictly require (Type) or (Type, String)
  before the factory closure, reject non-String/non-constant names
  with a clear error, and validate the resulting name is a legal Java
  identifier. Fixing this surfaced a real regression in my own first
  attempt - the common bean(Type, 'name') { ... } shape was briefly
  broken because the trailing closure is embedded in bean(...)'s own
  argument list when there is no .conditionalOnMissingBean(...)
  qualifier - caught by the existing test suite before it went
  anywhere.

- GenerateAutoConfigurationImportsTask caught every Throwable while
  loading scan candidates and did nothing with it, so a class that
  was genuinely annotated @AutoConfiguration but failed to load for a
  real reason would simply vanish from the generated imports file
  with no signal at all - its beans would never be registered and
  nothing would say why. Added an injectable callback, defaulting to
  a build warning in production; verified with a test that
  deliberately breaks a compiled class's superclass reference and
  confirms the callback fires with the right class name.

Verified: full test suites for grails-beans-dsl (22 tests, up from
17), grails-beans-dsl-example, grails-beans-dsl-plugin-example, and
build-logic (5 tests) all green; a third full-repo
`clean aggregateViolations :grails-test-report:check --continue` run
shows zero violations across CHECKSTYLE/CODENARC/PMD/SPOTBUGS with
fresh timestamps and the same pre-existing Docker-dependent failures
as the prior two runs, none in the touched modules; rat and
validateDependencyVersions both clean.

Report URL: https://github.com/apache/grails-core/actions/runs/29715921809

With regards,
GitHub Actions via GitBox

Reply via email to