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

fix: remove usage of deprecated Type attribute of resolver.Address #45

Closed
wants to merge 2 commits into from

Conversation

KnutZuidema
Copy link
Contributor

@KnutZuidema KnutZuidema commented Sep 19, 2023

The type attribute has been removed in more recent versions of gRPC. The usage here is not required and the may be removed.

See: grpc/grpc-go#6451
As mentioned in: #30 (comment)

The type attribute has been removed in more recent versions of gRPC. The usage here is not required and the may be removed.
@dannyskoog
Copy link

@KnutZuidema Probably makes sense to bump google.golang.org/grpc to the latest version as well. Plus remove the following line: https://github.com/sercand/kuberesolver/blob/master/builder_test.go#L44

@KnutZuidema
Copy link
Contributor Author

@dannyskoog agreed, however I'd prefer getting this immediately broken line fixed and then fixing the rest separately. I don't know what @sercand prefers. In the end I'm fine with either.

@dannyskoog
Copy link

@KnutZuidema Fair enough! Let's have @sercand decide that then

@sercand
Copy link
Owner

sercand commented Sep 19, 2023

@KnutZuidema Thanks for the PR. I think @dannyskoog is right, we should remove other usages of ".Type" in the repo but the version can stay same.

The type attribute has been removed in more recent versions of gRPC. The usage here is not required and the may be removed.

See: grpc/grpc-go#6451
@KnutZuidema
Copy link
Contributor Author

@sercand will close this PR and open another since using the master branch in my fork doesn't allow me to amend the requested addition into the existing commit.

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