Skip to content

Clean up duplicated cluster names#1001

Merged
istio-merge-robot merged 2 commits intoistio:masterfrom
qiwzhang:clean_up
Feb 7, 2018
Merged

Clean up duplicated cluster names#1001
istio-merge-robot merged 2 commits intoistio:masterfrom
qiwzhang:clean_up

Conversation

@qiwzhang
Copy link
Copy Markdown
Contributor

@qiwzhang qiwzhang commented Feb 6, 2018

What this PR does / why we need it:

Clean up some code

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

None

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 6, 2018
@qiwzhang qiwzhang mentioned this pull request Feb 6, 2018
Merged
JimmyCYJ
JimmyCYJ approved these changes Feb 6, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@JimmyCYJ JimmyCYJ left a comment

Choose a reason for hiding this comment

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

/lgtm

@lizan
Copy link
Copy Markdown
Contributor

lizan commented Feb 6, 2018

/lgtm

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 7, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@qiwzhang
Copy link
Copy Markdown
Contributor Author

qiwzhang commented Feb 7, 2018

/approve cancel
/lgtm cancel

@istio-testing istio-testing removed the lgtm label Feb 7, 2018
rshriram
rshriram approved these changes Feb 7, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@rshriram rshriram left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup. I would have done it in the other PR, but bot merged it before I could get to it.

@qiwzhang
Copy link
Copy Markdown
Contributor Author

qiwzhang commented Feb 7, 2018

transport_config pointer should not be re-used. In ReadV2Config() the http_config was passed to ParseFromJson. The transport object have been re-created after that. We were using the freed object.

@qiwzhang
Copy link
Copy Markdown
Contributor Author

qiwzhang commented Feb 7, 2018

PTAL

JimmyCYJ
JimmyCYJ approved these changes Feb 7, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@JimmyCYJ JimmyCYJ left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 7, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JimmyCYJ, lizan

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 7, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 7, 2018

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 2f36562 into istio:master Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lizan lizan Awaiting requested review from lizan

2 more reviewers

@rshriram rshriram rshriram approved these changes

@JimmyCYJ JimmyCYJ JimmyCYJ approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lizan lizan

@JimmyCYJ JimmyCYJ

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@qiwzhang @lizan @istio-merge-robot @rshriram @JimmyCYJ @googlebot @istio-testing