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

Conversation

@igorpeshansky
Copy link
Contributor

@igorpeshansky igorpeshansky commented Feb 2, 2018

As "infrastructureResource". Replaces "providerPlatform".

…ructureInstance". Replaces "providerPlatform".
@igorpeshansky
Copy link
Contributor Author

@Georgehe4 and @shoucongc FYI.

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.

LGTM

: config_(config), environment_(config) {}

json::value KubernetesReader::InstanceResource() const {
const std::string resource_type = "gce_instance"; // TODO: detect other resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

Only minor comment, does the comment need to wrap lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically it does, but it's a TODO that will be removed, and there are other lines in this file that are over 80 characters, so maybe overkill? TODOs that aren't line comments tend to get dissociated from the appropriate lines over time.

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.

LGTM

@igorpeshansky igorpeshansky merged commit a3d603d into master Feb 7, 2018
igorpeshansky added a commit that referenced this pull request Feb 7, 2018
As "infrastructureResource". Replaces "providerPlatform".
@igorpeshansky igorpeshansky deleted the igorp-kubernetes-instance-id branch February 7, 2018 21:26
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