mikebridge commented on PR #40128: URL: https://github.com/apache/superset/pull/40128#issuecomment-4604886742
Thanks @aminghadersohi for the re-review and the "ready to merge" verdict — appreciated. One follow-up pushed in 48ee9e6b51: **Migration `down_revision` update**: updated `9e1f3b8c4d2a`'s `down_revision` from `33d7e0e21daa` to `a1b2c3d4e5f6` (the current alembic head). Without this, `alembic upgrade head` would fail with "Multiple head revisions present" once the migration lands on a database that already has master's chain. The sibling charts (#40129) and datasets (#40130) PRs got the same update on their migrations in companion commits. Cross-PR merge-order coordination still applies — whichever lands first stays at `a1b2c3d4e5f6`, the others rebase onto the newly-merged revision. The branch was also rebased on current master in this push (the previous base was 34 commits behind preset/master; no conflicts). On the two advisory NITs: - **Test inline imports (Bucket 3)**: agreed — leaving them as-is, consistent with the prior round's classification. - **`_has_active_slug_twin` docstring framing**: good point on assertion-comment phrasing being more actionable than a forward-looking warning. I'll fold that into a follow-up cleanup commit if there's another reason to touch the file before merge; otherwise leaving it as a future-reader note. - **Restore returning `OK` instead of resource representation**: kept as-is for consistency with `bulk_delete`, agree it's a design choice the maintainers may want to weigh in on for the API contract. -- 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]
