oscerd commented on PR #24600: URL: https://github.com/apache/camel/pull/24600#issuecomment-4958608566
Post-merge review notes — the deep review of this PR finished after it merged, and most findings were already addressed by the pre-merge update and the follow-up commits (nice). Two things verified against current main that may be worth folding into the ongoing CAMEL-24003 follow-ups: 1. **`docs/user-manual/modules/ROOT/pages/camel-jbang-tui.adoc` now contradicts the shipped TUI**: the tab table still lists `| 3 | Diagram |` and `| 7 | Health |`, the text still says "The Diagram tab (Tab 3)" and "Health shows a badge when checks are DOWN" (the badge computation was removed in this PR), the new Activity tab isn't documented on that page, and the More-submenu list doesn't mention Health's relocation. Also worth a sentence somewhere user-facing: activity data only appears when the integration runs with backlog tracing enabled or standby — on a default run the tab/command is silently empty. 2. **`camel.trace.activityEnabled` doesn't actually gate activity capture**: in `core/camel-base-engine/.../impl/debugger/BacklogTracer.java` the field is only declared/get/set (lines 90/330/706), while `ActivityEventNotifier.isDisabled()` (line 819-820) checks only `!enabled && !standby` — so capture runs for any traced/standby app regardless of the flag, whose `main.adoc` description reads "Whether activity tracking is enabled". Either the notifier should consult the flag or the option description should be adjusted; happy to file a follow-up JIRA if useful. Tiny cosmetic one: `ActivityTab` help text still says "**Avg** / **Max** — elapsed time statistics" while the stats panel renders p50/p95/Max. For the record, the rest of the review came back clean: the earlier `ListActivity` `endpointUri`/`fromEndpointUri` mismatch is already fixed on main, hot-path capture is bounded and only exists when tracing is on (zero cost on default runs), activity data is metadata-only (no bodies/headers), the TUI `CopyOnWriteArrayList` wiring mirrors the existing `errors` pattern, generated command metadata/docs are regenerated, and CI was green. _Reviewed with Claude Code (Fable 5) on behalf of oscerd. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying._ -- 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]
