Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Icon): make "name" prop required #1723

Merged
merged 8 commits into from
Jul 30, 2019
Merged

Conversation

lucivpav
Copy link
Contributor

@lucivpav lucivpav commented Jul 26, 2019

An instantiated Icon without a name makes no sense. This PR makes the name prop mandatory. An alternative would be to set a default name value. This resolves issue #1715.

@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1723   +/-   ##
=======================================
  Coverage   70.98%   70.98%           
=======================================
  Files         860      860           
  Lines        7156     7156           
  Branches     2058     2078   +20     
=======================================
  Hits         5080     5080           
  Misses       2070     2070           
  Partials        6        6
Impacted Files Coverage Δ
packages/react/src/components/Icon/Icon.tsx 100% <ø> (ø) ⬆️
...test/specs/commonTests/implementsShorthandProp.tsx 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 aa1b907...6d852c6. Read the comment docs.

CHANGELOG.md Outdated Show resolved Hide resolved
@layershifter layershifter added 🧰 fix Introduces fix for broken behavior. 🚀 ready for review labels Jul 26, 2019
Co-Authored-By: Oleksandr Fediashov <[email protected]>
@vercel vercel bot temporarily deployed to staging July 29, 2019 15:15 Inactive
@lucivpav
Copy link
Contributor Author

lucivpav commented Jul 29, 2019

@layershifter can you please take a look at the new changes? I have fixed the test errors.

@lucivpav lucivpav merged commit b70bee7 into master Jul 30, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/icon-name-required branch July 30, 2019 10:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants