mikebridge commented on PR #41550: URL: https://github.com/apache/superset/pull/41550#issuecomment-5168290422
# Head moved since the re-review ping — delta summary for `faf478de47` @kgabryje — three commits landed after I asked for your re-review at `3cb75cd8b1`. All additive polish; none of your eight threads' code changed (two docstrings you reviewed were reworded, noted below). Summary so your pass can start at the current head: **`d7238b62a9`** — codeant's 08-03 round, both findings real ([threads resolved with replies](https://github.com/apache/superset/pull/41550#discussion_r3704557105)): `reconcile_pending()` now zeroes `removed_dashboard_slices` on **both** exit statuses, matching `finalize()`'s "asserts only what it witnessed" rule — the bot flagged `target_absent`; the reconcile `failed` branch had the identical gap and is fixed together, with both reconcile tests pinning the zeroing plus a new `target_absent`-shaped test. The purge Playwright spec gains the restore tests' `try/finally` cleanup. **`6fc57e645d`** — an internal three-reviewer pass (clean-code, tidy-first, and a react-review re-review — AI panels, findings verified against the tree before acting, same as the earlier rounds in this thread). Applied: `isSemanticView()` extracted to all seven DatasetList sites, replacing a "must match exactly" comment with a compiler-enforced function; a discriminating fixture test (semantic-view row with `kind` but no `source_type` — legal under the TS type; control run: the old `source_type` predicate fails it, `kind` passes 145/145); and the **`requireConfirmationText` prop deleted** from `DeleteModal`/`ConfirmStatusChange` — after your review restored the type-to-confirm gate its last caller went away, and shipping it would have put an unused optional prop into the published `@superset-ui/core` API. **`faf478de47`** — the same pass's follow-ups, folded in rather than ticketed: the `Archive`/`Delete` label and toast forks (~14 inline ternaries across the three list pages and `CRUD/utils`) now route through `softDeleteCopy.ts` helpers (each branch stays a whole translation unit; genuinely-different bulk strings stay inline); `handleRestore`/`handlePurge` collapse onto one `performRowAction` skeleton so the in-flight invariant is encoded once; and six comments narrating this branch's own review history ("used to guess UTC", "an earlier revision claimed…") are rewritten to timeless rationale — this is the comment-only rewording in `charts/filters.py` and `views/filters.py` docstrings you previously read. **Deliberately untouched:** your non-blocking architecture note on the triplicated purge route body. One of the internal reviewers wanted the `_run_purge(binding)` extraction done now; I've kept your recorded disposition — it's your call on re-review, and reversing it in backend files mid-pass seemed the wrong move. Verified at `faf478de47`: 329/329 across 22 Jest suites, `tsc --noEmit` clean, pre-commit green. CI running. -- 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]
