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

Move ValueTask to own namespace #440

Merged

Conversation

MarcelRoozekrans
Copy link
Contributor

  • Added nameValueTask params to differentiate when both namepsaces are imported.

Please note that the Result.Try ambigious test is impossible to fix.
My suggestion would be in this case if the valueTask named param is not an option we should remove one of the overloads.

- Added nameValueTask params to differentiate when both namepsaces are imported.
@vkhorikov vkhorikov merged commit 7497c42 into vkhorikov:master Aug 31, 2022
@vkhorikov
Copy link
Owner

Looks good. I've pushed a minor fix (directly to master) and also returned AmbiguityTests back so that they don't compile.

Regarding Result.Try, I suggest removing the overload with ValueTasks. We'll refactor it into extension methods sometime later. What do you think?

@MarcelRoozekrans
Copy link
Contributor Author

I agree with you! Problem with the .Try method is that its used statically from a partial class. And we cannot chare the partial class over namespaces this way it will never work.

Thoughts on the refactor because it uses static invocation I do not think it will work as an extension method :)

@vkhorikov
Copy link
Owner

Yeah, indeed. Not an extension method but a static method on a separate class from Result. Something like Try as @hankovich suggested.

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.

2 participants