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

Rename ColorScheme field 'name' to 'id' #35

Merged
merged 3 commits into from
Nov 17, 2018
Merged

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Nov 17, 2018

💔 Breaking Changes

BREAKING CHANGE: Rename ColorScheme field 'name' to 'id'

// from
new CategoricalColorScheme({ name: 'rgb', label: 'rgb colors', colors: ['red', 'green', 'blue']});
// to
new CategoricalColorScheme({ id: 'rgb', label: 'rgb colors', colors: ['red', 'green', 'blue']});

This was inspired by recent discussion in number-format PR #31 that it is confusing to have both name and label and the field name should indicate a bit more uniqueness.

@kristw kristw requested a review from a team as a code owner November 17, 2018 06:07
@kristw kristw added this to the v0.7.0 milestone Nov 17, 2018
@codecov
Copy link

codecov bot commented Nov 17, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #35   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          48     48           
  Lines         407    407           
=====================================
  Hits          407    407
Impacted Files Coverage Δ
...et-ui-color/src/colorSchemes/categorical/airbnb.js 100% <ø> (ø) ⬆️
...et-ui-color/src/colorSchemes/categorical/google.js 100% <ø> (ø) ⬆️
...perset-ui-color/src/colorSchemes/categorical/d3.js 100% <ø> (ø) ⬆️
...uperset-ui-color/src/colorSchemes/sequential/d3.js 100% <ø> (ø) ⬆️
...rset-ui-color/src/colorSchemes/categorical/lyft.js 100% <ø> (ø) ⬆️
...set-ui-color/src/colorSchemes/sequential/common.js 100% <ø> (ø) ⬆️
packages/superset-ui-color/src/ColorScheme.js 100% <100%> (ø) ⬆️
...superset-ui-color/src/CategoricalColorNamespace.js 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 8008606...7cfbb4d. Read the comment docs.

@kristw kristw force-pushed the kristw--color-scheme-id branch from 004f369 to 433b0a9 Compare November 17, 2018 06:10
@kristw kristw added the reviewable Ready for review label Nov 17, 2018
Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

LGTM! I agree this is better, one other spot that could be changed is the storybook ... it wouldn't break but now references a field that doesn't exist.

@kristw kristw merged commit cf4f821 into master Nov 17, 2018
@delete-merged-branch delete-merged-branch bot deleted the kristw--color-scheme-id branch November 17, 2018 19:09
@kristw kristw removed reviewable Ready for review labels Nov 19, 2018
@kristw kristw added #code-quality and removed reviewable Ready for review labels Dec 6, 2018
kristw added a commit that referenced this pull request Apr 17, 2020
* build: use shared commit config

* docs: changelog

* fix: root package.json version
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants