Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore:migrate ResultSet from class components to functional components #18821

Conversation

The-hyphen-user
Copy link
Contributor

SUMMARY

changed class components to functional

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

testing file still needs to be updated to test use-effect instead of component did mount/ component will receive props

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@superset-github-bot superset-github-bot bot added the Superset-Community-Partners Preset community partner program participants label Feb 19, 2022
@EugeneTorap
Copy link
Contributor

@The-hyphen-user Hi, we already have the same #18593 PR
You can help me to finish my #18729 #18593 #18148 PRs

displayLimit,
height,
query,
search,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this has a default value, which you should put in. Same with database, csv, showSql, and visualize.

);
};

ResultSet.defaultProps = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can move this in favor of stating defaults in the functional component itself.

@stale
Copy link

stale bot commented Apr 27, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 27, 2022
@kgabryje
Copy link
Member

kgabryje commented Sep 7, 2022

Superseded by #21186.

@kgabryje kgabryje closed this Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Inactive for >= 30 days size/XL Superset-Community-Partners Preset community partner program participants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants