jdaugherty commented on PR #16025:
URL: https://github.com/apache/grails-core/pull/16025#issuecomment-5835338143

   @matrei thanks for the review. Everything is addressed except the sign-off 
in item 1, which is still open. Changes are in `d19c00c9db` through 
`1d6d2396d1`.
   
   **1. `actions/*` SHAs vs 7.0.x**
   - 7.0.x is aligned first, as you suggested.
     - `ci/unpin-first-party-actions-7.0.x` replaces all 97 `actions/*` SHA 
pins on 7.0.x.
     - It applies the same `AGENTS.md` rule change.
   - Both branches reference each `actions/*` action by the full tag of its 
latest release: `[email protected]`, `[email protected]`, `[email protected]`, 
`[email protected]`, `[email protected]`. The ASF allows every 
version in that namespace.
   - The gate now also rejects floating major tags such as `@v7` for 
`actions/*`. `apache/*` still accepts any version or branch.
   - @jamesfredley, can you confirm you agree with this policy?
   
   **2. The profile jar ships the wrapper**
   - The description now explains the fix: the old copy source was 
`install/grails-wrapper`, which never exists.
   - It is also listed under a new Release notes section.
   
   **3. Writers can publish a stale report**
   - `writeStyleViolations` and `writeAnalysisViolations` are no longer in the 
`verification` group.
   - A writer requested without its aggregate task is now skipped, including 
from the configuration cache.
   - I kept them as separate tasks rather than folding them into the aggregate. 
Under `--continue`, the aggregate doesn't run once an analyzer fails, and the 
writer still has to produce the report that explains the failure.
   
   **4. `enablePmd()` / `enableSpotbugs()`**
   - Done, following the `withSourcesJar()` pattern you described.
     - Both methods apply and configure the tool immediately.
     - The `-P` properties are read in `apply()`, and an all-project `false` 
makes both methods no-ops.
     - Report locations and the build-directory exclude resolve lazily.
   - The `afterEvaluate` caveat is gone from both docs.
   - New specs cover:
     - direct `tasks.named('pmdMain')` customization
     - a `reportsDirectory` set after `enablePmd()`
     - applying the plugin from `gradle.projectsEvaluated`
     - the full property table for both tools
   
   **5. Nits**
   - Report names are readable: `__grails-datasource-pmdMain.xml`.
     - A project path that doesn't decode back to itself fails the build, so 
names can't collide.
     - Test reports are now recognized by the task name alone. Otherwise 
`grails-testing-support-core` would be misread as a test report.
   - The unrelated `—` → `-` edits in `AGENTS.md` are reverted.
   - The description no longer says "again" for `zip -y`.
   - The ZIP layout change has a Release notes line.
   - The docs now say the all-project property, when set, wins over both 
`.projects` and the module opt-ins. There's also a table row where `false` is 
combined with both.
   


-- 
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]

Reply via email to