davsclaus commented on PR #29663:
URL: https://github.com/apache/camel/pull/29663#issuecomment-6820335259

   LGTM. One small suggestion for a test, not blocking.
   
   I checked that a built-in plugin such as `tui` is no longer loaded for 
`camel run` / `camel export`. On main today it is, because `CliExportContext` 
calls `getActivePlugins(main, repos)` with no target. With this PR, 
`effectiveAppliesTo` falls through to `PluginType.TUI`, which has no 
`appliesTo`. That returns an empty list, not `null`, so the plugin is skipped 
even with `legacyContributors = true`. The same holds for kubernetes, generate, 
edit, validate and mcp. 👍
   
   `knownPluginTypeAppliesToIsUsedAtRuntime` covers the positive case (`test` 
is loaded for `export`). Nothing pins the negative case: a known plugin with an 
empty `appliesTo` (e.g. a config entry for `tui`) is **not** loaded by 
`getActivePlugins(main, null, "run", true)`. That is the path that keeps the 
TUI and friends out of every `run`/`export`, and a later change to 
`effectiveAppliesTo` (say, returning `null` for an empty enum entry) would 
quietly bring them back. A test like 
`knownPluginWithoutAppliesToIsSkippedForRunWithLegacyContributors` would guard 
it.
   
   Also, a small wording point on the upgrade-guide NOTE: "annotation default 
`appliesTo = {}` means loaded for run/export" holds only for third-party 
plugins. For the built-in Camel plugins, the `PluginType` enum decides first, 
so they stay restricted to their own command.
   
   _Claude Code on behalf of davsclaus_
   


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