Skip to content

docs(#3534): add review guidance for dynamic-plugins.yaml minimality - #3536

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/3534-dynamic-plugins-review-guidance
Open

docs(#3534): add review guidance for dynamic-plugins.yaml minimality#3536
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/3534-dynamic-plugins-review-guidance

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add a "Reviewing dynamic-plugins.yaml Changes" subsection to the E2E Testing section of AGENTS.md, after "Configuration Files". This gives the review agent explicit criteria to flag unnecessary entries in dynamic-plugins.yaml files:

  • Entries setting disabled: true for plugins outside the workspace
    own metadata (redundant, auto-generation only includes workspace
    plugins)
  • Entries duplicating what auto-generation would produce from
    metadata appConfigExamples
  • Guidance that only entries overriding specific test-environment
    values justify maintaining the file

This addresses a gap identified during NFS (app-next) migration reviews where legacy config entries carry forward unnecessarily.


Closes #3534

Post-script verification

  • Branch is not main/master (agent/3534-dynamic-plugins-review-guidance)
  • Secret scan passed (gitleaks — 102e37162b04bc906dbe8dd95e4d7414c0da5339..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add a "Reviewing dynamic-plugins.yaml Changes" subsection to the E2E
Testing section of AGENTS.md, after "Configuration Files". This gives
the review agent explicit criteria to flag unnecessary entries in
dynamic-plugins.yaml files:

- Entries setting disabled: true for plugins outside the workspace
  own metadata (redundant, auto-generation only includes workspace
  plugins)
- Entries duplicating what auto-generation would produce from
  metadata appConfigExamples
- Guidance that only entries overriding specific test-environment
  values justify maintaining the file

This addresses a gap identified during NFS (app-next) migration
reviews where legacy config entries carry forward unnecessarily.

Closes #3534
@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown

Hi @fullsend-ai-coder[bot]. Thanks for your PR.

I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:46 PM UTC · Completed 4:00 PM UTC

Commit: c0a54b0 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $3.10

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [internal-consistency] AGENTS.md:316 — Line 316 states "ALL metadata files are read to auto-generate the complete plugin configuration," while the new text clarifies that auto-generation only includes plugins from the workspace's own metadata/*.yaml files. Investigation of the implementation (readWorkspacePackages in smoke-tests-native/src/workspace.ts) confirms the new text is factually correct — auto-generation is workspace-scoped. The existing phrasing on line 316 is contextually correct but could be slightly clearer by saying "all of the workspace's metadata files" instead of "ALL metadata files." Optional follow-up improvement.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • AGENTS.md (file-level): Line 316 · [low] internal-consistency

Line 316 states 'ALL metadata files are read' which could be misread as cross-workspace, but in context it clearly means 'all metadata files for that workspace.' The new PR text is factually correct (confirmed by implementation in smoke-tests-native/src/workspace.ts readWorkspacePackages). Optional clarity improvement: amend line 316 to say 'all of the workspace metadata files are read.'

Suggested fix: Optional: amend line 316 from 'ALL metadata files are read' to 'all of the workspace metadata files are read' to eliminate any ambiguity.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 2, 2026
@github-actions github-actions Bot added the non-workspace-changes PR changes files outside workspace directories label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test non-workspace-changes PR changes files outside workspace directories requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add AGENTS.md review guidance for dynamic-plugins.yaml minimality in NFS test configs

0 participants