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

feat: add functions for parsing formatters from encoding #205

Merged
merged 4 commits into from
Aug 20, 2019

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Aug 14, 2019

🏆 Enhancements

  • Add functions for parsing formatters from encoding

@kristw kristw requested a review from a team as a code owner August 14, 2019 23:11
@netlify
Copy link

netlify bot commented Aug 14, 2019

Deploy preview for superset-ui ready!

Built with commit 24e5a14

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

@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

Merging #205 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files         107      111       +4     
  Lines        1222     1239      +17     
  Branches      297      303       +6     
==========================================
+ Hits         1221     1238      +17     
  Partials        1        1
Impacted Files Coverage Δ
...set-ui-encodeable/src/parsers/fallbackFormatter.ts 100% <100%> (ø)
...eable/src/parsers/createFormatterFromChannelDef.ts 100% <100%> (ø)
...c/parsers/createFormatterFromFieldTypeAndFormat.ts 100% <100%> (ø)
...codeable/src/parsers/createGetterFromChannelDef.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 b53b839...24e5a14. Read the comment docs.

@kristw kristw added the reviewable Ready for review label Aug 15, 2019
Copy link

@isaacpz isaacpz left a comment

Choose a reason for hiding this comment

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

LGTM

@kristw kristw force-pushed the kristw--encodeable2 branch from 694f02f to 666bd61 Compare August 15, 2019 23:22
@kristw kristw closed this Aug 16, 2019
@kristw kristw reopened this Aug 16, 2019
Copy link

@isaacpz isaacpz left a comment

Choose a reason for hiding this comment

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

Still LGTM, just a small typing nit

import { ChannelDef } from '../types/ChannelDef';
import { isValueDef } from '../typeGuards/ChannelDef';

export default function createGetterFromChannelDef(definition: ChannelDef): (x?: any) => any {
Copy link

Choose a reason for hiding this comment

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

Could this function have more explicit typing? If not, would be good to leave a comment documenting expected input/output

@espressoroaster
Copy link

LGTM as well

@kristw kristw merged commit 8906aa0 into master Aug 20, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--encodeable2 branch August 20, 2019 18:29
kristw pushed a commit that referenced this pull request Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewable Ready for review size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants