Skip to content
This repository was archived by the owner on Jan 16, 2022. It is now read-only.

hooks first step#115

Merged
juanpicado merged 4 commits intomasterfrom
hooks-first-step
Aug 15, 2019
Merged

hooks first step#115
juanpicado merged 4 commits intomasterfrom
hooks-first-step

Conversation

@juanpicado
Copy link
Copy Markdown
Member

@juanpicado juanpicado commented Aug 12, 2019

Type: refactor

The candidate here is src/components/Developers/Developers.tsx.
I extracted the Avatar to src/components/AvatarTooltip/AvatarTooltip.tsx 🆕

The following has been addressed in the PR:

  • There is a related issue?
  • Unit or Functional tests are included in the PR yes

Description:

First try using hooks. Some notes:

  • By now, only useState and useContext usage.
  • Enzyme support hooks if you mount the component.
  • @testing-library/react might be useful in coordination with Enzyme.
  • We can move to hooks only components if update the test to mount
  • It might be verbose due material-ui using mount.

"@material-ui/core": "3.9.3",
"@material-ui/icons": "3.0.2",
"@octokit/rest": "16.28.7",
"@testing-library/react": "9.1.0",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am adding this @testing-library/react for experimenting using the render method.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 12, 2019

Codecov Report

Merging #115 into master will increase coverage by 2.37%.
The diff coverage is 95.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   71.28%   73.66%   +2.37%     
==========================================
  Files          88       89       +1     
  Lines         874      877       +3     
  Branches      156      157       +1     
==========================================
+ Hits          623      646      +23     
+ Misses        239      219      -20     
  Partials       12       12
Impacted Files Coverage Δ
src/components/Developers/styles.ts 100% <ø> (ø) ⬆️
src/components/Developers/Developers.tsx 100% <100%> (+100%) ⬆️
src/components/Icon/styles.ts 90% <100%> (ø) ⬆️
src/components/AvatarTooltip/AvatarTooltip.tsx 83.33% <83.33%> (ø)

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 542212a...3d7b230. Read the comment docs.

Copy link
Copy Markdown
Contributor

@sergiohgz sergiohgz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@juanpicado juanpicado removed the wip label Aug 15, 2019
@juanpicado juanpicado merged commit d81b610 into master Aug 15, 2019
@juanpicado juanpicado deleted the hooks-first-step branch August 15, 2019 19:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants