Correct comments of deleteFreeContainer and add corresponding test #679
Conversation
|
Can one of the admins verify this patch? |
| @@ -505,7 +505,8 @@ func (rc *realModel) updateFreeContainer(ce *cache.ContainerElement) (time.Time, | |||
| return latest_time, err | |||
| } | |||
|
|
|||
| // updateFreeContainer updates Free Container-level information from a ContainerElement | |||
| // deleteFreeContainer deletes a free container from the belonging node. | |||
| // deleteFreeContainer receives a host name of the belonging node, and a name of the free container. | |||
There was a problem hiding this comment.
not: returns rather than receives
There was a problem hiding this comment.
ignore me, misread sorry
|
LGTM |
|
ok to test |
1 similar comment
|
ok to test |
|
Looks like jenkins is down, or at least unreachable for me: http://104.154.52.74/job/heapster-e2e-gce/ Either way, the only change is a unit test, which are by Travis. Jenkins isn't really needed for this PR, merging. |
Correct comments of deleteFreeContainer and add corresponding test
|
Jenkins should be accessible here. On Fri, Oct 30, 2015 at 5:44 AM, Daniel Martí notifications@github.com
|
|
@vishh |
|
We run a test server to validate PRs. My previous comment was for @mvdan. On Fri, Oct 30, 2015 at 1:51 PM, Jianwei Shi notifications@github.com
|
|
@vishh |
|
@bluebreezecf: Thanks for offering help! This PR is fine as-is. |
|
@vishh: With my pleasure bro! |
I read into sources of heapster, and find out the comments of deleteFreeContainer in model/impl.go are just copy from the method updateFreeContainer, which will make confusion and be meaningless.
So I correct the comments of deleteFreeContainer (), and add the corresponding method test in model/impl_test.go.