bito-code-review[bot] commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3488876045
##########
superset-frontend/src/features/allEntities/AllEntitiesTable.tsx:
##########
@@ -59,7 +59,7 @@ interface AllEntitiesTableProps {
}
export default function AllEntitiesTable({
- search = '',
+ search: _search = '',
Review Comment:
<!-- Bito Reply -->
The suggestion to remove the unused prop is technically correct regarding
dead code, but your assessment that this is a behavior-adjacent trim is
reasonable. Since the prop is already unused on the master branch, renaming it
to `_search` is a standard lint-compliant way to acknowledge the unused
variable without altering the interface or the caller. You may choose to ignore
this suggestion if you prefer to keep the interface stable for now.
**superset-frontend/src/features/allEntities/AllEntitiesTable.tsx**
```
export default function AllEntitiesTable({
search: _search = '',
```
##########
superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx:
##########
@@ -118,7 +118,6 @@ interface MatrixifyGridRendererProps {
function MatrixifyGridRenderer({
formData,
datasource,
- width,
height,
hooks,
}: MatrixifyGridRendererProps) {
Review Comment:
<!-- Bito Reply -->
The suggestion to remove `width` from the `MatrixifyGridRendererProps`
interface is appropriate. Since `width` is no longer used within the component
body, removing it from the interface correctly aligns the component's API
contract with its implementation.
**superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx**
```
interface MatrixifyGridRendererProps {
formData: any;
datasource: any;
height: number;
hooks: any;
}
```
##########
superset-frontend/src/dashboard/components/gridComponents/TabsRenderer/TabsRenderer.tsx:
##########
@@ -170,7 +169,6 @@ const TabsRenderer = memo<TabsRendererProps>(
tabBarPaddingLeft = 0,
onTabsReorder,
isEditingTabTitle = false,
- onTabTitleEditingChange,
}) => {
const [activeId, setActiveId] = useState<string | null>(null);
Review Comment:
<!-- Bito Reply -->
The removal of the unused prop is appropriate. Since the callback was not
being invoked and the prop was not wired to any functionality, removing the
destructuring cleans up the code without introducing a regression.
**superset-frontend/src/dashboard/components/gridComponents/TabsRenderer/TabsRenderer.tsx**
```
tabBarPaddingLeft = 0,
onTabsReorder,
isEditingTabTitle = false,
}) => {
const [activeId, setActiveId] = useState<string | null>(null);
```
##########
superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx:
##########
@@ -60,7 +60,6 @@ export const SamplesPane = ({
queryFormData,
queryForce,
setForceQuery,
- isVisible,
canDownload,
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to remove `isVisible` from the component's props
is locally correct as it cleans up an unused prop. However, as you noted, the
prop remains in the type definition and is still passed by the caller. While
this is technically a type contract mismatch, it is common to handle such
cleanups in phases. Given that this PR is focused on removing unused code, your
approach of trimming the component first is acceptable, provided that a
follow-up task is created to clean up the type definition and callers to avoid
leaving dead surface area.
**superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx**
```
export const SamplesPane = ({
queryFormData,
queryForce,
setForceQuery,
canDownload,
```
--
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]
