-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Update Container.ts to be more testable #205
Conversation
Figure if im going to be a contributor to the package, i should at least contribute more than a one line typing change :) |
@jmclean-cnexus this is great! Moving the utility functions into a separate file really helps with reducing complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just reviewed with a few quick requested changes. Once those are fixed, I'll build, format, and then merge! Thanks for the great contribution!
@jmclean-cnexus perfect! I'll merge and release a new version on npm this evening after I get back from work. Thanks again! 😄 |
@jmclean-cnexus just merged and published as version 8.0.8 🚀 |
Break out the container.ts code so utility functions are imported, and can be tested in isolation. Added some more unit tests for the low hanging fruit utility functions