Skip to content
New issue

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

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

Already on GitHub? # to your account

refactor: migrate ExploreCtasResultsButton component to typescript #18142

Merged
merged 14 commits into from
Feb 17, 2022
Merged

refactor: migrate ExploreCtasResultsButton component to typescript #18142

merged 14 commits into from
Feb 17, 2022

Conversation

EugeneTorap
Copy link
Contributor

SUMMARY

Migrated ExploreCtasResultsButton to TypeScript to apply direction outlined in #18100 .

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue: Migrate components to TypeScript #18100
  • 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

@EugeneTorap
Copy link
Contributor Author

@lyndsiWilliams Can you review this PR?

Copy link
Member

@geido geido left a comment

Choose a reason for hiding this comment

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

Hey @EugeneTorap this generally looks good. However, you have some linting issues to fix for the CI to pass. Let me know when you update this PR and I'll have a look at it. Thanks!

@EugeneTorap
Copy link
Contributor Author

Hey @EugeneTorap this generally looks good. However, you have some linting issues to fix for the CI to pass. Let me know when you update this PR and I'll have a look at it. Thanks!

@geido Can you merge #17939 first and then I will update my PR

@geido
Copy link
Member

geido commented Feb 9, 2022

Hey @EugeneTorap this generally looks good. However, you have some linting issues to fix for the CI to pass. Let me know when you update this PR and I'll have a look at it. Thanks!

@geido Can you merge #17939 first and then I will update my PR

Hello @EugeneTorap the #17939 is now merged. Please resolve your conflicting files and I'll be happy to do another review. Thanks!

# Conflicts:
#	superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.jsx
@EugeneTorap EugeneTorap changed the title refactor: migrate ExploreCtasResultsButton component to FC & tsx refactor: migrate ExploreCtasResultsButton component to typescript Feb 9, 2022
@pull-request-size pull-request-size bot added size/M and removed size/L labels Feb 9, 2022
@EugeneTorap EugeneTorap requested a review from geido February 9, 2022 11:55
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #18142 (7a4de6f) into master (225015f) will decrease coverage by 0.04%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18142      +/-   ##
==========================================
- Coverage   66.28%   66.23%   -0.05%     
==========================================
  Files        1605     1605              
  Lines       62863    62817      -46     
  Branches     6341     6341              
==========================================
- Hits        41666    41606      -60     
- Misses      19545    19559      +14     
  Partials     1652     1652              
Flag Coverage Δ
javascript 51.27% <33.33%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...frontend/src/SqlLab/components/ResultSet/index.tsx 50.73% <ø> (ø)
...lLab/components/ExploreCtasResultsButton/index.tsx 8.33% <33.33%> (ø)
superset/connectors/connector_registry.py 78.33% <0.00%> (-3.34%) ⬇️
superset/security/manager.py 92.30% <0.00%> (-1.96%) ⬇️
superset/utils/core.py 89.77% <0.00%> (-0.51%) ⬇️
superset/connectors/sqla/models.py 88.29% <0.00%> (-0.48%) ⬇️
superset/db_engine_specs/base.py 88.43% <0.00%> (-0.32%) ⬇️
superset/db_engine_specs/trino.py 70.53% <0.00%> (-0.27%) ⬇️
superset/db_engine_specs/mssql.py 95.83% <0.00%> (-0.09%) ⬇️
superset/charts/data/api.py 89.80% <0.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 225015f...7a4de6f. Read the comment docs.

@geido
Copy link
Member

geido commented Feb 10, 2022

/testenv up

@github-actions
Copy link
Contributor

@geido Ephemeral environment spinning up at http://54.212.233.219:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@EugeneTorap
Copy link
Contributor Author

@AAfghahi @lyndsiWilliams Can you review this PR?

@suddjian suddjian merged commit 8dc2377 into apache:master Feb 17, 2022
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@EugeneTorap EugeneTorap deleted the ExploreCtasResultsButton-to-tsx branch February 17, 2022 21:25
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants