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

feat(option/internaloption): add WithDefaultEndpointTemplate #2313

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

quartzmo
Copy link
Member

  • Add DefaultEndpointTemplate to internal/settings.go
  • Deprecate internaloption.WithDefaultEndpoint

refs: #2264

* Add DefaultEndpointTemplate to internal/settings.go
* Deprecate internaloption.WithDefaultEndpoint

refs: googleapis#2264
@quartzmo quartzmo requested a review from a team as a code owner December 18, 2023 22:03
@quartzmo quartzmo requested a review from shollyman December 18, 2023 22:35
@@ -22,10 +22,29 @@ func (o defaultEndpointOption) Apply(settings *internal.DialSettings) {
// It should only be used internally by generated clients.
//
// This is similar to WithEndpoint, but allows us to determine whether the user has overridden the default endpoint.
//
// Deprecated: WithDefaultEndpoint does not support setting the universe domain.
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to not just repurpose this option. I think we could since it is internal?

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually plan to use both options simultaneously during a transitional period as universe domain support is incrementally expanded.

Copy link
Member

Choose a reason for hiding this comment

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

could you do the same thing with one and just check for a %s? I think either way is fine as long as we plan to fully remove the old one eventually

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely plan to remove the old one.

@quartzmo quartzmo merged commit 9e6e0c7 into googleapis:main Dec 19, 2023
5 checks passed
@quartzmo quartzmo deleted the option-default-template branch December 19, 2023 18:25
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