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

Deprecate Dial and DialContext through a mechanism that doesn't trigger linters #7235

Closed
howardjohn opened this issue May 15, 2024 · 1 comment

Comments

@howardjohn
Copy link
Contributor

howardjohn commented May 15, 2024

See #7090.

IMO, the change here was technically correct (see #7090 (comment)) but impractical for the broader ecosystem. Typically deprecations like this are done in more obscure functions that have relatively small impact; I have never seen (in the Go ecosystem), such an impactful deprecation before.

As #7090 (comment) notes, there are 178,000 usages across the open source ecosystem of these functions. With this change, all of them will need to be updated (if not to NewClient, to silence the linting). This is placing a substantial burden on developers for little benefit to them. Isolating to specific orgs, Kubernetes and Istio both have well over 100 usages of these methods.

I don't feel its required for a function to be formally // Deprecated: (which triggers the linters) to discourage usage. As prior art, in both Go stdlib and gRPC-go itself, DialContext was added to ~replace Dial. In both of these cases, the other was not formally deprecated.

I propose the comment on these functions is tweaked to not trigger linters.

howardjohn added a commit to howardjohn/grpc-go that referenced this issue May 15, 2024
@dfawley
Copy link
Member

dfawley commented May 15, 2024

there are 178,000 usages

The go stdlib ioutil package, deprecated in 2022, is still imported by almost 700k packages: https://pkg.go.dev/io/ioutil?tab=importedby

We're following all the guidelines here as best we can. I'm sorry this has inconvenienced you, but you probably do need a way to allow the use of deprecated features in your presubmits, so you don't have to immediately update >100 call sites at the same time you update a dependency. For example, we have a list of exceptions here:

grpc-go/scripts/vet.sh

Lines 123 to 125 in 0020ccf

# Only ignore the following deprecated types/fields/functions and exclude
# generated code.
noret_grep "(SA1019)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused

An explanation of this change and why we made it can be found in https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. Let us know if you have any questions or other concerns about it.

@dfawley dfawley closed this as completed May 15, 2024
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 a pull request may close this issue.

2 participants