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

Make --server_port optional in C++ interop client #25550

Merged
merged 3 commits into from
Feb 26, 2021

Conversation

apolcyn
Copy link
Contributor

@apolcyn apolcyn commented Feb 24, 2021

This is to allow running the interop client against services that only require a hostname as the channel target.

Also motivated by b/181142141

@apolcyn apolcyn added lang/c++ release notes: no Indicates if PR should not be in release notes labels Feb 24, 2021
@apolcyn apolcyn marked this pull request as ready for review February 25, 2021 19:36
absl::GetFlag(FLAGS_server_host).c_str(),
absl::GetFlag(FLAGS_server_port));

std::string host_port = absl::GetFlag(FLAGS_server_host);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just call this host instead of host_port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about server_uri? (changed to that)

Preferred that over "host" since host may sound like it includes only the hostname component of the uri

@apolcyn
Copy link
Contributor Author

apolcyn commented Feb 26, 2021

ios binary size: #25431
Macos Dbg C/C++ hasn't completed running, but merging since last test queue time was ~20 hours ago, and this has change has low risk of breaking macos dbg (macos opt tests passed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/c++ release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants