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

Update DisposeFieldsAndPropertiesInTearDownAnalyzer.cs to support Quit() #803

Closed
wants to merge 1 commit into from

Conversation

rhyous
Copy link

@rhyous rhyous commented Dec 5, 2024

Update DisposeFieldsAndPropertiesInTearDownAnalyzer.cs to support Quit() as in Selenium that calls Dispose().

Update DisposeFieldsAndPropertiesInTearDownAnalyzer.cs to support Quit() as that calls Dispose().
@rhyous
Copy link
Author

rhyous commented Dec 5, 2024

@dotnet-policy-service agree company="Rhyous"

@mikkelbu
Copy link
Member

mikkelbu commented Dec 5, 2024

Hi @rhyous

Thanks for the contribution. As far as I can remember we chose the current method names to match the CA2000 rule. I'm not against adding Quit to the list, but are there any special case you are considering? As far as I can tell Quit is used by selenium-webdriver, but I'm unsure if there are other cases.

@rhyous
Copy link
Author

rhyous commented Dec 5, 2024

Cool.

Yes, I am using Selenium. I also don't know of any other cases.

FYI, I checked for any pull requests for adding Quit() already, in case it had already been rejected, and I didn't see one. So I posted this suggestion/pull request and the nunit team could either say yes, this is good, or no.

For now, I'm suppressing this. I'll accept whatever you think is best.

@manfred-brands
Copy link
Member

This seems to be specific to a library you use.
Have you tried setting dotnet_diagnostic.NUnit1032.additional_dispose_methods: Quit in your .editorconfig?

@rhyous
Copy link
Author

rhyous commented Dec 6, 2024

@m

This seems to be specific to a library you use. Have you tried setting dotnet_diagnostic.NUnit1032.additional_dispose_methods: Quit in your .editorconfig?

I didn't try that. I found how to suppress it in the docs and used that first. I will try this suggestion out. Thanks!

Update: That suggestion worked. I like it better as it doesn't suppress. I would say the existing feature to allow for adding additional dispose methods is great. My apologies for not seeing that in the docs.

@mikkelbu
Copy link
Member

@rhyous No need for apologies. We have forgotten to add this information to the docs. I'll close this PR and then open a ticket about adding this to the docs of NUnit1032

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