-
Notifications
You must be signed in to change notification settings - Fork 41
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
Update sync examples with check
to async check
#1448
Conversation
a0a453d
to
18eb2dd
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.
Thanks for working on this change.
My personal preference would be to avoid then
chaining as i think it looks complicated to a new user who is new to JS.
I'd also only try to retrieve data to compare within a check
rather than do any changes to the DOM or elsewhere in the website/system. I like to think that a check
is almost like an assert, and generally we only compare two things in an assert, rather than perform state changes to the system within the assert.
Since these are both style issue though, happy to approve and get them merged in 👍
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.
Thanks for doing this. 🙏
I left a few comments.
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.
Thanks! LGTM 🚀
What?
Updates sync examples with
check
to asynccheck
.Some of the examples don't require an async check, but it's better to use async
check
in them for consistency.Why?
The sync k6
check
is cumbersome when working with async functions.Checklist
Related PR(s)/Issue(s)
grafana/k6-docs#1725