Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: add more support for undefined format to number and time formatters #308

Merged
merged 3 commits into from
Mar 4, 2020

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Mar 4, 2020

🏆 Enhancements

The code used to be format || defaultFormat, and it was changed by eslint auto-fix to format ?? defaultFormat.

'' || 'abc' = 'abc'
'' ?? 'abc' = ''

@kristw kristw requested a review from a team as a code owner March 4, 2020 21:29
Copy link
Contributor

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

lgtm!

@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

Merging #308 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #308   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         103    103           
  Lines        1223   1225    +2     
  Branches      300    300           
=====================================
+ Hits         1223   1225    +2
Impacted Files Coverage Δ
...ber-format/src/NumberFormatterRegistrySingleton.ts 100% <ø> (ø) ⬆️
...-time-format/src/TimeFormatterRegistrySingleton.ts 100% <ø> (ø) ⬆️
...et-ui-number-format/src/NumberFormatterRegistry.ts 100% <100%> (ø) ⬆️
...perset-ui-time-format/src/TimeFormatterRegistry.ts 100% <100%> (ø) ⬆️

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 af3c35c...4f9db76. Read the comment docs.

@netlify
Copy link

netlify bot commented Mar 4, 2020

Deploy preview for superset-ui ready!

Built with commit 4f9db76

https://deploy-preview-308--superset-ui.netlify.com

@pull-request-size pull-request-size bot added size/M and removed size/S labels Mar 4, 2020
formatterId === null || typeof formatterId === 'undefined' || formatterId === ''
? this.defaultKey
: formatterId
}`.trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate the explicitness, but could this be simplified as formatterId ?? (formatterId || this.defaultKey).

Copy link
Contributor Author

@kristw kristw Mar 4, 2020

Choose a reason for hiding this comment

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

eslint will try to change || to ??

@kristw kristw merged commit 44ad062 into master Mar 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the kristw--fix-undef branch March 4, 2020 22:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants