gkneighb commented on PR #43567: URL: https://github.com/apache/superset/pull/43567#issuecomment-5882847837
@rusackas Thanks for the offer. Nothing outstanding here is blocking, but I did push one substantive change since your comment, at `235a683df5`. `funnel.py` had the same `model_dump()` shape you flagged on #43573, so it now uses `exclude_unset=True` with a `test_merge_funnel_preserves_omitted_defaults` regression beside the pie twin. I audited every registered plugin for it — the table is on #43573; gantt looked guilty to a grep but is actually fine. On @aminghadersohi's three notes, all of which he marked non-blocking: the thin-coverage one is fair (every test uses the default `row_limit=10` and none sets `color_scheme`, so mutants survive) and I'm happy to add the non-default fixture he describes if you'd like it in this PR. The other two — `_funnel_chart_what` being byte-identical to `_pie_chart_what`, and `sort_by_metric` resetting on update — are both pre-existing for pie as well, so I'd rather not change shared behavior under a plugin PR. Say the word on the fixture and I'll push it; otherwise this is ready from my side. -- 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]
