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

Make the "Async" suffix optional globally not just for test methods #63

Open
girtsl opened this issue Aug 14, 2020 · 1 comment
Open

Comments

@girtsl
Copy link

girtsl commented Aug 14, 2020

This extension offers a neat refactoring - to convert a method to async - and this is the reason I found it after getting tired of changing the method signatures manually and started to look for an automated solution. However, this refactoring also does other things, without the possibility to turn them off:

  • it automatically adds the suffix "Async";
  • changes all inner calls to their async counterparts, if such exist.

I'm aware that the "Async" suffix is a MS convention:

You should add "Async" as the suffix of every async method name you write.

This is the convention used in .NET to more easily differentiate synchronous and asynchronous methods. Certain methods that aren't explicitly called by your code (such as event handlers or web controller methods) don't necessarily apply. Because they are not explicitly called by your code, being explicit about their naming isn't as important.

This might have been more relevant back when those guidelines were first written, but as some, me included, have come to think, only adds noise to the code. Take this comment - #31 (comment) - for an example, or the excellent write-up by Particular Software - https://docs.particular.net/nservicebus/upgrades/5to6/async-suffix.

I thought this extension would save me time but now I need to manually rename methods back by removing the "Async" suffix, making it a deal-breaker.
For the other refactoring - changing all inner calls to their async counterparts - I have not yet encountered a case where it has lead to an unwelcome result, but only because I've not used it on code that has such async counterparts. Still, it would be nice to be able to turn it off.

Thank you for considering this proposal.

@dmcbride
Copy link

Honestly, all of the options here should be individually configurable to be test-only or universal:

ConfigureAwait
Async suffix
Eliding await

Obviously, turning these all off universally means that maybe this isn't the extension for you, and that's fine. But there are some aspects here which would really help, and others that are difficult to integrate into our codebase, so it would be really helpful if those that are difficult could be turned off, and we can see the positives easier.

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

No branches or pull requests

2 participants