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

chore(style): Enforce optional chaining #21614

Merged
merged 3 commits into from
Oct 3, 2022
Merged

Conversation

rusackas
Copy link
Member

SUMMARY

Just saw this comment on a PR today. I see and write comments like this all the time, so let's just make it a rule!

This PR turns on enforcement around optional chaining. It also fixes the 58 or so errors that resulted.

This rule does NOT have an auto-fix option, so I did all these by hand. Please check my work!

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Linting should not fail, CI should pass.

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

@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #21614 (9853ffc) into master (b8c1aa5) will increase coverage by 0.10%.
The diff coverage is 36.36%.

@@            Coverage Diff             @@
##           master   #21614      +/-   ##
==========================================
+ Coverage   66.71%   66.82%   +0.10%     
==========================================
  Files        1796     1799       +3     
  Lines       68716    69223     +507     
  Branches     7313     7471     +158     
==========================================
+ Hits        45843    46255     +412     
- Misses      21011    21064      +53     
- Partials     1862     1904      +42     
Flag Coverage Δ
hive 52.90% <ø> (ø)
javascript 53.27% <36.36%> (+0.33%) ⬆️
mysql 78.21% <ø> (ø)
postgres 78.27% <ø> (ø)
presto 52.80% <ø> (ø)
python 81.40% <ø> (ø)
sqlite 76.77% <ø> (ø)
unit 50.91% <ø> (ø)

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

Impacted Files Coverage Δ
...chart-controls/src/shared-controls/dndControls.tsx 58.33% <0.00%> (ø)
...superset-ui-core/src/connection/callApi/callApi.ts 100.00% <ø> (ø)
...rts/src/BigNumber/BigNumberTotal/transformProps.ts 0.00% <0.00%> (ø)
...BigNumber/BigNumberWithTrendline/transformProps.ts 47.05% <0.00%> (ø)
...d/src/SqlLab/components/AceEditorWrapper/index.tsx 57.25% <0.00%> (+9.56%) ⬆️
...rset-frontend/src/components/Chart/chartReducer.ts 25.49% <0.00%> (+0.49%) ⬆️
...ntend/src/components/ConfirmStatusChange/index.tsx 90.47% <ø> (-0.44%) ⬇️
...uperset-frontend/src/components/FacePile/index.tsx 91.66% <ø> (-1.20%) ⬇️
superset-frontend/src/components/Select/utils.tsx 81.48% <0.00%> (ø)
...Filters/FilterBar/FilterControls/FilterControl.tsx 29.03% <0.00%> (ø)
... and 60 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@EugeneTorap
Copy link
Contributor

LGTM! @rusackas can you fix CI errors?

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

This PR gave me great satisfaction, thanks for making this a rule! ❤️

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

LGTM. Really great improvements to code readability!

Copy link
Member

@stephenLYZ stephenLYZ left a comment

Choose a reason for hiding this comment

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

LGTM!

@rusackas rusackas merged commit 4245bc3 into master Oct 3, 2022
@rusackas rusackas deleted the enforce-optional-chaining branch October 3, 2022 15:17
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 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/L 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants