mikebridge commented on PR #41551: URL: https://github.com/apache/superset/pull/41551#issuecomment-5168087823
*Generated by Claude (AI) on behalf of @mikebridge.* **Head correction for round 3: one commit landed after the response above — re-pin to `136d0fc826`.** After posting, we ran a four-lens local review panel (clean-code / tidy-first / continuous-delivery / react) over the full branch. 22 unique findings, no blockers; the four confirmed small ones are in `136d0fc826`: - `openRelatedEntity` called `window.open` after awaiting the id lookup — the silent Safari popup-block failure `navigationUtils`' own docstring warns about, and which `openAsNew` already avoided. It now claims the tab synchronously (two lenses found this independently). - The restore/fork in-flight locks were per-hook-instance refs, but the preview banner and the history panel each mount their own `useVersionActions` — banner "Open as new" plus panel kebab mid-flight forked two copies. Now a module-scope lock, entity-wide for restore, per-version for fork. The new two-instance regression test forks twice against the old code. - The `?version_history=true` effects re-ran on every `canRestore` move, so a late false→true flip re-opened a panel the user had closed. Honoured once per mount, both dashboard and explore sides. - Save groups with no `versionUuid` offered kebab actions whose clicks silently no-op'd; the kebabs are now hidden for such groups. Also for your structural read: the panel independently converged on the preview-orchestration decomposition — that's now four lenses (your rounds, CD, clean-code, tidy-first) pointing at the same hook, which we take as further corroboration for the `listenerMiddleware` flag-on gate proposed above rather than a reason to restructure mid-review. One reviewer re-raised the withdrawn `canOverwriteSlice` owner-branch claim as a High — flagging it here so it doesn't cost you time: the PR description already documents the withdrawal and the `#38831` model-history evidence in the SUMMARY and the disposition table. Nothing new there. Suites 166/166 in the feature, tsc and pre-commit clean at `136d0fc826`. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
