Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(package): use keyboard-key package #2707

Merged
merged 5 commits into from
Apr 30, 2018
Merged

Conversation

levithomason
Copy link
Member

@levithomason levithomason commented Apr 6, 2018

We'll be extracting more of our /lib to separate packages for use in other frameworks. This PR abstracts the keyboard-key util.

@brianespinosa
Copy link
Member

@levithomason Is the plan to make this into a monorepo that exports read-only versions of these lib packages so they can be imported elsewhere?

@levithomason
Copy link
Member Author

Yes, that is the plan for the components. I hadn't considered it for the lib, but that also may make sense for some things.

Did you have a proposal on this by chance?

@brianespinosa
Copy link
Member

Did you have a proposal on this by chance?

No proposal on this yet. I am still exploring options on doing this in my company's current projects. Depending on how this gets set up there's potentially a lot of infrastructure to set up... but once it is done you get the benefits of both the many/singe repo approach. I don't have any experiences to share at this time but once I've done some trial runs and a POC in my own project I can open up a RFC for this on our project and we can have a discussion about our needs and what further info I might have from my experiences.

@codecov-io
Copy link

codecov-io commented Apr 30, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2707      +/-   ##
==========================================
- Coverage   99.74%   99.74%   -0.01%     
==========================================
  Files         160      160              
  Lines        2759     2758       -1     
==========================================
- Hits         2752     2751       -1     
  Misses          7        7
Impacted Files Coverage Δ
src/addons/Pagination/PaginationItem.js 100% <ø> (ø) ⬆️
src/modules/Search/Search.js 99.46% <100%> (-0.01%) ⬇️
src/modules/Rating/RatingIcon.js 100% <100%> (ø) ⬆️
src/modules/Dropdown/Dropdown.js 100% <100%> (ø) ⬆️
src/addons/Portal/Portal.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 94e5684...35cea1a. Read the comment docs.

@layershifter
Copy link
Member

@levithomason I've finished this PR, take a look 🐱

@levithomason
Copy link
Member Author

Excellent, we can merge this now 💪

Moving forward, I think we might want to consider making our lib a mono repo with yarn workspaces rather than having literal separate repos. Thoughts?

@levithomason levithomason merged commit d57b054 into master Apr 30, 2018
@levithomason levithomason deleted the chore/keyboard-key branch April 30, 2018 23:02
@levithomason
Copy link
Member Author

Released in semantic-ui-react@0.80.0.

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

Successfully merging this pull request may close these issues.

4 participants