Skip to content

Handle name when creating a container and returning list of containers#55

Merged
bkendall merged 7 commits intoRunnable:masterfrom
doganyazar:master
Dec 25, 2015
Merged

Handle name when creating a container and returning list of containers#55
bkendall merged 7 commits intoRunnable:masterfrom
doganyazar:master

Conversation

@doganyazar
Copy link
Copy Markdown
Contributor

I just wanted to add functionality to get container's name from /containers/create api.
I do not know why but Docker api returns each container's name in "Names" field when listing containers via /containers/json?all=1
(https://docs.docker.com/engine/reference/api/docker_remote_api_v1.21/#list-containers)

@bkendall
Copy link
Copy Markdown
Contributor

bkendall commented Dec 4, 2015

nice! could you create tests for these new bits of functionality?

@doganyazar
Copy link
Copy Markdown
Contributor Author

Yeah I should have done that :) I will definitely do that in a couple of days when I am done with my current task.

@doganyazar
Copy link
Copy Markdown
Contributor Author

Ok I have just added a check to make sure same container name cannot be used. And added test for it and also for the original Names field.
Not sure why the coverage failed.

@cflynn07
Copy link
Copy Markdown
Contributor

@doganyazar looks like your changes dropped the coverage levels below our minimum thresholds.

From npm run coverage

ERROR: Coverage for statements (81.09%) does not meet global threshold (82%)
ERROR: Coverage for lines (80.9%) does not meet global threshold (82%)
ERROR: Coverage for functions (83.33%) does not meet global threshold (87%)

@bkendall
Copy link
Copy Markdown
Contributor

@doganyazar the coverage is run strictly on the test/models and test/utils folders. You need to add unit tests for ConflictError at minimum. I can take a closer look later today.

@doganyazar
Copy link
Copy Markdown
Contributor Author

Coverage seems fine now :) Do you think we can merge this guys?

@cflynn07
Copy link
Copy Markdown
Contributor

Thanks @doganyazar !

@cflynn07
Copy link
Copy Markdown
Contributor

Just going to wait until @bkendall has a chance to review this

@bkendall
Copy link
Copy Markdown
Contributor

this looks good, @doganyazar. thanks for the tests.

bkendall added a commit that referenced this pull request Dec 25, 2015
Handle name when creating a container and returning list of containers
@bkendall bkendall merged commit a99d132 into Runnable:master Dec 25, 2015
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