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

Move Components interface to common package #213

Merged
merged 7 commits into from
Aug 19, 2016
Merged

Conversation

jcscottiii
Copy link
Member

@jcscottiii jcscottiii commented Aug 19, 2016

This patch is just #212 broken into smaller pieces

This patch moves the components interface to the common package
Also, it get rids of the GetAndApply function that passes a callback
Also, it adds the mock for the component

  • The Component mock needed to be added in this PR because of the tests coverage was complaining due to coverage lowering and the mock helped with making tests

This patch does not unexport all the functions because the workspace
interface has not been created yet. Will come in another PR
A future PR will introduce instructions on how to generate the mocks.

This patch moves the components interface to the common package
Also, it get rids of the GetAndApply function that passes a callback
This patch does not unexport all the functions because the workspace
interface has not been created yet. Will come in another PR
@codecov-io
Copy link

codecov-io commented Aug 19, 2016

Current coverage is 92.03% (diff: 95.74%)

Merging #213 into master will decrease coverage by 0.53%

@@             master       #213   diff @@
==========================================
  Files            26         27     +1   
  Lines           888        879     -9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            822        809    -13   
- Misses           52         56     +4   
  Partials         14         14          

Powered by Codecov. Last update a0746d3...7f619a8

@jcscottiii
Copy link
Member Author

@afeld this is ready

components.Lock()
defer components.Unlock()
added := false
if _, exists := components.mapping[component.GetKey()]; !exists {
Copy link
Member

Choose a reason for hiding this comment

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

While I know this isn't unusual for Go, I'm personally not a fan of cramming assignment plus a check into a conditional. I favor one thing per line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@jcscottiii
Copy link
Member Author

@afeld this should have touched on everything. and threw in some extra tests too lol

@afeld afeld merged commit cbe89a9 into master Aug 19, 2016
@afeld afeld deleted the move-components-interface branch August 19, 2016 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants