Skip to content
This repository was archived by the owner on Aug 19, 2019. It is now read-only.

Conversation

@igorpeshansky
Copy link
Contributor

No description provided.

Copy link
Contributor

@bmoyles0117 bmoyles0117 left a comment

Choose a reason for hiding this comment

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

I like these changes! LGTM

Copy link
Contributor

@supriyagarg supriyagarg left a comment

Choose a reason for hiding this comment

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

A minor comment. Otherwise these changes look great!

@@ -1157,12 +1160,15 @@ void KubernetesUpdater::StartUpdater() {
config().KubernetesClusterLevelMetadata() ? "" : current_node);

// Wrap the bind expression into a function to use as a bind argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this comment to mention lambda expression rather than bind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment is outdated. I just removed it.

Copy link
Contributor Author

@igorpeshansky igorpeshansky left a comment

Choose a reason for hiding this comment

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

PTAL.

@@ -1157,12 +1160,15 @@ void KubernetesUpdater::StartUpdater() {
config().KubernetesClusterLevelMetadata() ? "" : current_node);

// Wrap the bind expression into a function to use as a bind argument.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment is outdated. I just removed it.

@supriyagarg
Copy link
Contributor

Thanks. LGTM

@igorpeshansky igorpeshansky merged commit 724873f into igorp-kubernetes-unscheduled-pods Mar 13, 2018
@igorpeshansky igorpeshansky deleted the igorp-lambdas-replace-std-bind branch March 13, 2018 21:56
@igorpeshansky
Copy link
Contributor Author

Sigh. Merged into the wrong branch. It's ok, we'll pick this up once #73 is merged.

igorpeshansky added a commit that referenced this pull request Mar 18, 2018
This is a cherry-pick of #74 into master.
igorpeshansky added a commit that referenced this pull request Mar 18, 2018
This is effectively a cherry-pick of #74 into master.
igorpeshansky added a commit that referenced this pull request Mar 19, 2018
This is effectively a cherry-pick of #74 into master.
@igorpeshansky
Copy link
Contributor Author

Superseded by #83.

igorpeshansky added a commit that referenced this pull request Mar 22, 2018
igorpeshansky added a commit that referenced this pull request Mar 23, 2018
…own files. (#87)

* Replace an std::bind instance missed in #74.
* Stop using MetadataAgent as a wrapper around config+store.
* Replace includes by forward declarations in headers whenever possible.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants