-
Notifications
You must be signed in to change notification settings - Fork 15
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
Misc release hotfixes #986
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #986 +/- ##
=======================================
- Coverage 23% 16% -6%
=======================================
Files 143 201 +58
Lines 4355 6361 +2006
=======================================
+ Hits 959 986 +27
- Misses 3396 5375 +1979 |
de6e0b1
to
36c4a5f
Compare
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.
I just have one question regarding the use of unawaited
. Everything else looks good to me!
a3f3b4c
to
a259460
Compare
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.
Well done!
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.
@bdmendes an |
@limwa it sure does, but is that related to the issue @LuisDuarte1 is having? |
No, I don't think so, it was just something that came across while testing. |
I will test this further, but on other note, I think that the profile provider shouldn't have a timeout (or it should have a timeout greater than the rest of the providers) since it's needed at startup, this should reduce the problem with slow internet connections. |
I also suggest, if it's easier to implement, to set the timeout on this provider (or all providers) to 30 seconds. Should be more than enough, even for slow connections. |
bc54384
to
2c82d64
Compare
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.
LGTM 🚀
d001a18
to
59558fc
Compare
Closes #993
Closes #994
Review checklist
whatsnew/whatsnew-pt-PT
changelog.md
with the change