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.

Sign up for GitHub

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

Jump to bottom

Call ConfigureAwait(false) on all library related await calls #2500

Merged
merged 13 commits into from
Dec 30, 2019
Merged

Call ConfigureAwait(false) on all library related await calls #2500

merged 13 commits into from
Dec 30, 2019

Conversation

javiercampos
Copy link
Contributor

Related to #2497

Only added the ConfigureAwait(false) calls on library solutions (all Volo.Abp.* solutions).

All tests pass except for 1 but doesn't seem related to this. The failing one is:
Volo.Abp.Emailing.EmailTemplateStore_Tests.Should_Get_Registered_Template_With_Layout

Which fails with:

   Shouldly.ShouldAssertException : template.Content
    should contain (case insensitive comparison)
"<body>
    Please confirm your email address by clicking the link below."
    but was actually
"<!DOCTYPE html>
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">

Doesn't seem related to any of these commits and should probably be fixed separately

@javiercampos javiercampos mentioned this pull request Dec 29, 2019
Closed
@javiercampos
Copy link
Contributor Author

All commits are 1 per solution (except the fixing one, which the analyzer couldn't figure out correctly)

@hikalkan
Copy link
Member

That's a great contribution. Thank you for your huge effort.

@hikalkan hikalkan added this to the 2.0 milestone Dec 30, 2019
@maliming
Copy link
Member

@mehmetuken mentioned a https://github.com/Fody/ConfigureAwait class library. It can save a lot of code. I haven't tried it, but from the introduction, it really is.

@javiercampos
Copy link
Contributor Author

@maliming adding Fody to the build pipeline might be painful... when it works, it works fine: when it doesn't, you'll want to pull all your hairs off... I'd GREATLY recommend AGAINST it for a library. I find it more than ok for app code (I've used it in the past for many things), but not for a library :-)

@hikalkan
Copy link
Member

@maliming Fody seems good, yes, but I also think to not add a new dependency for now. I will merge this PR and delay the Fody decision.

@hikalkan hikalkan merged commit 0a15687 into abpframework:dev Dec 30, 2019
@hikalkan hikalkan mentioned this pull request Jan 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
abp-framework enhancement
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@javiercampos @hikalkan @maliming