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

api: add BindOptions.NonRecursive #2780

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

AkihiroSuda
Copy link
Member

From moby/moby#38003

Signed-off-by: Akihiro Suda [email protected]

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@08d6e4a). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #2780   +/-   ##
=========================================
  Coverage          ?   61.94%           
=========================================
  Files             ?      137           
  Lines             ?    22131           
  Branches          ?        0           
=========================================
  Hits              ?    13709           
  Misses            ?     6947           
  Partials          ?     1475

@AkihiroSuda
Copy link
Member Author

@dperny PTAL?

@dperny
Copy link
Collaborator

dperny commented Nov 26, 2018

LGTM. Sorry, was on vacation last week.

@AkihiroSuda
Copy link
Member Author

cc @wk8 @anshulpundir

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Thanks @AkihiroSuda - could you update the other dependencies as well? (see below)

vendor.conf Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Member Author

updated

@@ -9175,6 +9192,10 @@ file {
name: "ConfChangeUpdateNode"
number: 2
}
value {
name: "ConfChangeAddLearnerNode"
Copy link
Member

Choose a reason for hiding this comment

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

This was added in etcd-io/etcd#8751, related to etcd-io/etcd#8568

wondering if we need local changes to take the new type into account (e.g. #1711)

(should be ok as follow up)

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @dperny

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, but left one comment for a possible follow-up

@dperny
Copy link
Collaborator

dperny commented Jan 7, 2019

Going back, I'm confused as to why there are so many vendoring updates included with this PR

@thaJeztah
Copy link
Member

Going back, I'm confused as to why there are so many vendoring updates included with this PR

These are just updated to match the versions that are used with the current version of moby/moby. Train of thought there is that CI in this repository will be testing against the same versions of the dependencies than it will ultimately be used with when vendoring swarmkit in moby/moby.

See #2780 (comment) above; the initial version of this PR only updated the moby/moby vendor itself (and CI was green with that)

@dperny
Copy link
Collaborator

dperny commented Jan 8, 2019

Alright, looks good to me.

@dperny dperny merged commit 87d4106 into moby:master Jan 8, 2019
@dperny
Copy link
Collaborator

dperny commented Jan 8, 2019

Sorry to everyone who's PRs will now be broken and need a rebase.

@thaJeztah
Copy link
Member

Don't think there's other PR's doing a vendor update, so we should be fine

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