Skip to content

chore: remove enums from project#266

Merged
markov00 merged 5 commits intoelastic:masterfrom
markov00:remove_enums
Jul 17, 2019
Merged

chore: remove enums from project#266
markov00 merged 5 commits intoelastic:masterfrom
markov00:remove_enums

Conversation

@markov00
Copy link
Copy Markdown
Collaborator

@markov00 markov00 commented Jul 16, 2019

Summary

This PR removes all the remaining enums on the project as babel doesn't support const enum directly.
The refactoring doesn't create any breaking change, just a bit more verbose code.

@emmacunningham I've also seen a problem with the AnnotationType , not resolved in this PR but I can work on it:

export const AnnotationTypes = Object.freeze({
  Line: 'line' as AnnotationType,
  Rectangle: 'rectangle' as AnnotationType,
  Text: 'text' as AnnotationType,
});

export type AnnotationType = 'line' | 'rectangle' | 'text';

in this way, if you specify something like

const annotation = AnnotationTypes.Line

the type of annotation is AnnotationType and not line that means that if you want to do typechecking on that variable needs to go in a different way. I've fixed that behaviour specifying the exact type for every object value like Bottom: 'bottom' as 'bottom' and I've reused the typeof Position.Bottom instead of rewriting the string type. Let me know what do you think

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • [ ] This was checked for cross-browser compatibility, including a check against IE11
  • [ ] Proper documentation or storybook story was added for features that require explanation or tutorials
  • Unit tests were updated or added to match the most common scenarios
  • Each commit follows the convention

@markov00 markov00 added the chore label Jul 16, 2019
@markov00 markov00 requested review from emmacunningham and nickofthyme and removed request for nickofthyme July 16, 2019 17:55
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 16, 2019

Codecov Report

Merging #266 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
- Coverage   98.12%   98.11%   -0.01%     
==========================================
  Files          36       37       +1     
  Lines        2662     2654       -8     
  Branches      607      621      +14     
==========================================
- Hits         2612     2604       -8     
  Misses         45       45              
  Partials        5        5
Impacted Files Coverage Δ
src/lib/series/curves.ts 100% <100%> (ø) ⬆️
src/lib/utils/interactions.ts 100% <100%> (ø) ⬆️
src/lib/themes/theme.ts 100% <100%> (ø) ⬆️
src/state/chart_state.ts 97.45% <100%> (ø) ⬆️
src/lib/series/specs.ts 100% <100%> (ø) ⬆️
src/lib/utils/scales/scales.ts 100% <100%> (ø)
src/state/annotation_utils.ts 100% <0%> (ø) ⬆️
src/lib/series/domains/y_domain.ts 100% <0%> (ø) ⬆️
src/lib/series/domains/x_domain.ts 100% <0%> (ø) ⬆️
src/lib/series/scales.ts 100% <0%> (ø) ⬆️
... and 6 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 a8f99b1...c3bfa74. Read the comment docs.

Copy link
Copy Markdown
Contributor

@emmacunningham emmacunningham left a comment

Choose a reason for hiding this comment

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

code lgtm. the approach of explicitly assigning each value to itself and then using typeof in the union type definition looks good and can be extended for the annotation types.

@markov00 markov00 changed the title Remove enums from project chore: remove enums from project Jul 17, 2019
@markov00 markov00 merged commit ebe9d3f into elastic:master Jul 17, 2019
@markov00 markov00 deleted the remove_enums branch July 17, 2019 15:57
@markov00
Copy link
Copy Markdown
Collaborator Author

🎉 This PR is included in version 8.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore released Issue released publicly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants