-
Notifications
You must be signed in to change notification settings - Fork 391
Translation and BigQuery implement IDisposable #1624
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
Conversation
5e0aeff
to
a1fd12d
Compare
/// </remarks> | ||
public abstract partial class BigQueryClient | ||
public abstract partial class BigQueryClient : IDisposable |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@benwulfe: No, because it's not actually a requirement. As documented, it's something you should do in some situations, but it's not only allowing you to be more efficient. Any code that doesn't dispose of the client will work just as well as it did before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an extra client to do: AdvancedTranslationClient.
|
If you have singletons that implement IDisposable, not disposing it is a pretty common (if maybe not entirely correct) usage pattern. I'd argue this caveat doesn't really make it special or unique. |
I guess all I was saying is that the very existence of having an object that implements IDisposable that is not disposed is likely to create a lint error where it was ok before. |
Well, I'd argue that the expected behaviour at that point is for the developer to read the linter error, read the doc comment then, and configure the linter appropriately. If the linter can't be configured, I think that's a bigger problem. (And how does it cope with HttpClient and Task for example?) I really believe that bumping the major version number would be a mistake here, giving an unrealistic impression of breaking. |
e84d2a7
to
1fd47c7
Compare
No description provided.