rusackas commented on PR #39528:
URL: https://github.com/apache/superset/pull/39528#issuecomment-4434742703

   A few things to address here:
   
   1) Pre-commit is failing
   2) No tests added... would love to see something covering the new callback 
prop or the refreshKey increment logic.
   3) In `ChartTable.tsx`, the PR wraps `refreshData` in a new 
`handleRefreshData` arrow function to tack on the callback, then passes 
`refreshData={handleRefreshData}` to the inner component. This works, but it 
slightly diverges from the `DashboardTable` approach, where 
`onActivityRefresh?.()` is called directly at the call site. The two files 
should ideally follow the same pattern for consistency.


-- 
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]

Reply via email to