-
Notifications
You must be signed in to change notification settings - Fork 143
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
Updates usage of co
to async function in createScoreboardClient
#644
Conversation
Codecov Report
@@ Coverage Diff @@
## master #644 +/- ##
==========================================
- Coverage 81.07% 80.62% -0.45%
==========================================
Files 156 156
Lines 4776 4780 +4
Branches 184 184
==========================================
- Hits 3872 3854 -18
- Misses 855 877 +22
Partials 49 49
Continue to review full report at Codecov.
|
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.
Unfortunately for this one I have to request some changes. Hope you don’t mind 😅!
// TODO [#632]: Convert the `signUp` method to async function (instead of using `co`) in src/online/scoreboard-system/createScoreboardClient.js | ||
// See issue #575 for more details. | ||
return co(function*() { | ||
const { idToken } = yield* authenticationFlow.signUp( |
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.
This one is a bit tricky because we’re using yield*
instead of yield
. Simply changing it to await
doesn't work as we will get back an iterator instead of a Promise. To really fix this you have to convert generator functions in authenticationFlow
to async functions. Would you help do that for me?
invariant(typeof username === 'string', 'username must be a string') | ||
invariant(typeof password === 'string', 'password must be a string') | ||
// TODO [#633]: Convert the `loginByUsernamePassword` method to async function (instead of using `co`) in src/online/scoreboard-system/createScoreboardClient.js | ||
return co(function*() { | ||
const { idToken } = yield* authenticationFlow.loginByUsernamePassword( |
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.
(Ditto)
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.
This looks great now, many thanks!!
Kudos, SonarCloud Quality Gate passed!
|
fixes #632 and #633