Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

remove Register return value#30753

Merged
LK4D4 merged 1 commit intomoby:mastermoby/moby:masterfrom
NickrenREN:daemon-registerNickrenREN/docker:daemon-registerCopy head branch name to clipboard
Feb 6, 2017
Merged

remove Register return value#30753
LK4D4 merged 1 commit intomoby:mastermoby/moby:masterfrom
NickrenREN:daemon-registerNickrenREN/docker:daemon-registerCopy head branch name to clipboard

Conversation

@NickrenREN
Copy link
Copy Markdown
Contributor

Since Register() will never return err,remove the return value

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Feb 6, 2017
Since Register() will never return err,remove the return value

Signed-off-by: NickrenREN <yuquan.ren@easystack.cn>
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Feb 6, 2017
Comment thread daemon/container.go
@@ -98,8 +98,6 @@ func (daemon *Daemon) Register(c *container.Container) error {

daemon.containers.Add(c.ID, c)
daemon.idIndex.Add(c.ID)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking at these, it appears that we're ignoring any error that can occur when adding to the index; https://github.com/docker/docker/blob/fe9f5610c49b1a6ca2ebbb68c1478fc7f106eaa8/pkg/truncindex/truncindex.go#L59-L81

wondering if we should actually return those errors instead

/cc @cpuguy83

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would say we've never checked errors here before, and the errors that could be produced are already checked earlier on before we send to truncindex.
The only really valid error is if something couldn't be inserted, but in the end is not actionable to the user, and does not affect the operation of the container in anyway, just lookup by prefix.

It probably wouldn't hurt, but it won't help either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, thanks!

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@LK4D4 LK4D4 left a comment

Choose a reason for hiding this comment

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

LGTM

@LK4D4 LK4D4 merged commit b11f988 into moby:master Feb 6, 2017
@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.