Skip to content

Cleanup links top level pkg#14773

Merged
tiborvass merged 1 commit intomoby:masterfrom
runcom:cleanup-links
Jul 30, 2015
Merged

Cleanup links top level pkg#14773
tiborvass merged 1 commit intomoby:masterfrom
runcom:cleanup-links

Conversation

@runcom
Copy link
Member

@runcom runcom commented Jul 20, 2015

  • remove container.activeLinks not really used anywhere
  • wondering if top level links pkg make sense anymore since it's only doing ToEnv and NewLink
  • DisableLink was only used in daemon/delete and apart from updating activeLinks (not necessary) it was calling container.UpdateNetwork(), now container.UpdateNetwork() is called directly in daemon/delete
  • Move top level pkg links under daemon (but I guess we remove it also)

related to #14756 (if links pkg remains)

Signed-off-by: Antonio Murdaca runcom@linux.com

@runcom
Copy link
Member Author

runcom commented Jul 20, 2015

ping @LK4D4

@icecrime icecrime mentioned this pull request Jul 20, 2015
Closed
@LK4D4
Copy link
Contributor

LK4D4 commented Jul 20, 2015

@runcom Is windows fail related to your PR?

@runcom
Copy link
Member Author

runcom commented Jul 20, 2015

@LK4D4 seems totally unrelated with a weird error

FAIL: docker_api_containers_test.go:1553: DockerSuite.TestPostContainersCreateWithStringOrSliceEntrypoint

docker_api_containers_test.go:1562:
    c.Assert(strings.TrimSpace(out), check.Equals, "hello world")
... obtained string = "" +
...     "2015/07/20 22:49:23 error generating version 4 uuid, retrying: open /dev/urandom: operation not permitted\n" +
...     "hello world"
... expected string = "hello world"

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 20, 2015

Hah, no luck

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 21, 2015

Lol, I give up, three different errors on three runs.

@runcom
Copy link
Member Author

runcom commented Jul 21, 2015

@LK4D4 found the issue on windows, will fix (though errors were not helpful at all :/)

fixed but windows CI shows green even with 2 tests failed and this is happening on other PR as well

@runcom runcom force-pushed the cleanup-links branch 3 times, most recently from d981aed to 052eb3c Compare July 23, 2015 09:45
@runcom
Copy link
Member Author

runcom commented Jul 24, 2015

ping @LK4D4 @cpuguy83 @duglin

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 28, 2015

@runcom Need rebase :/

@runcom
Copy link
Member Author

runcom commented Jul 28, 2015

@LK4D4 rebased

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 28, 2015

LGTM

gdevillele
gdevillele reviewed Jul 29, 2015
View reviewed changes
daemon/links/links.go Outdated
Copy link
Contributor

@gdevillele gdevillele Jul 29, 2015

Choose a reason for hiding this comment

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

add period a the end of the func comment

@runcom
Cleanup links top level pkg
0e0c7e5 
- Move top level pkg links under daemon
- Refactor code accordingly
- golint pkg

Signed-off-by: Antonio Murdaca <runcom@linux.com>
@gdevillele
Copy link
Contributor

gdevillele commented Jul 29, 2015

Thanks.
LGTM

@tiborvass
Copy link
Contributor

tiborvass commented Jul 30, 2015

CI passed except for two known tests that are already fixed by now, so LGTM!

tiborvass added a commit that referenced this pull request Jul 30, 2015
@tiborvass
Merge pull request #14773 from runcom/cleanup-links
6adb64e 
Cleanup links top level pkg
@tiborvass tiborvass merged commit 6adb64e into moby:master Jul 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@runcom @LK4D4 @gdevillele @tiborvass @GordonTheTurtle