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

Create unmock-types package #358

Merged
merged 16 commits into from Dec 17, 2019
Merged

Create unmock-types package #358

merged 16 commits into from Dec 17, 2019

Conversation

@k4m4
Copy link
Contributor

k4m4 commented Dec 14, 2019

Fixes #357

Description

Creates a separate lerna package called unmock-types that contains commonly used types (from interfaces.ts).

@k4m4 k4m4 changed the title Unmock types Create unmock-types package Dec 14, 2019
@k4m4 k4m4 requested review from ksaaskil and carolstran Dec 14, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 14, 2019

Codecov Report

Merging #358 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #358   +/-   ##
=======================================
  Coverage   80.39%   80.39%           
=======================================
  Files          54       54           
  Lines        2413     2413           
  Branches      583      583           
=======================================
  Hits         1940     1940           
  Misses        473      473

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 ad1e314...29e23dc. Read the comment docs.

Copy link
Contributor

ksaaskil left a comment

Looks great, thanks! Please consider squash merging when merging to keep the commit history clean, thanks 🙂

Copy link
Contributor

carolstran left a comment

Looks great generally, thanks for taking care 🎉

Only note is that it would be great if we could have a bit more documentation on the README about what unmock-types does - even pulling some information from the original issue into a short description would be helpful.

As of now, we haven't been great overall about documenting our packages on unmock.io - but now this is inspiring me to open some related issues/tasks 🤔🤨😄

@k4m4

This comment has been minimized.

Copy link
Contributor Author

k4m4 commented Dec 17, 2019

@carolstran I totally agree. Although, in this particular example, I am not entirely sure what else we could include in the docs. This package just contains commonly used types, which is what is currently denoted in the readme. Any particular ideas, before we merge this?

@carolstran

This comment has been minimized.

Copy link
Contributor

carolstran commented Dec 17, 2019

@k4m4 Oh sorry, I should've been more clear - my comment shouldn't block merging! It was more of a general note, but I'm planning on opening some documentation-related issues later so we don't need to address it in this PR.

Merge away 🚀

@k4m4

This comment has been minimized.

Copy link
Contributor Author

k4m4 commented Dec 17, 2019

@carolstran Got it, thanks!

@k4m4 k4m4 merged commit 020233e into dev Dec 17, 2019
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: verify-before-publish Your tests passed on CircleCI!
Details
@k4m4 k4m4 deleted the unmock-types branch Dec 17, 2019
@ksaaskil

This comment has been minimized.

Copy link
Contributor

ksaaskil commented Dec 19, 2019

The package is now available in npm! I'll probably use those only in diglett in the short run but we can slowly work on migrating also unmock-js to use the package. I'm planning to write open up discussion how the whole unmock-js could be refactored to be more modular, so that'll probably involve some planning what should be included in unmock-types (like IInterceptor, IFaker etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.