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
Decaff querying.coffee and its test. #5643
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
2bcc9be
to
d6984d0
Compare
server-e2e-tests-electron-5 failed because of a flaky test. I ran the test 5+ times on my 18.04 ubuntu. It all passed. |
d6984d0
to
3d4b597
Compare
Failed because of a flaky test. |
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.
Please make sure to read through each line of converted code (also make sure eslint is on to catch simple mistakes). We want to maintain readability of the code, which is why converting to JS has not been a quick process of just convert and go.
I made some suggestions and highlighted some areas where the conversion was less than ideal for readability.
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.
Please make sure to read through each line of converted code (also make sure eslint is on to catch simple mistakes). We want to maintain readability of the code, which is why converting to JS has not been a quick process of just convert and go.
I made some suggestions and highlighted some areas where the conversion was less than ideal for readability.
Fixed eslint errors and removed returns.
4a427e2
to
bd32628
Compare
Fixed eslint errors and removed returns. Minor fixes. Fixed switches. Removed returns.
bd32628
to
d115aa7
Compare
@jennifer-shehane Thanks for the long review. Based on your review, I've created a checklist here. |
@jennifer-shehane Ah, I forgot to mention this. Everything you told me is fixed and merged into the last commit. |
@sainthkh This is so great 😍 @bahmutov Thoughts on adding this at a PR comment when there is a PR with commit messages prepended with |
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.
Great! Thank you!
Released in |
Preparation for fixing #92.
NEVER SQUASH THIS PR.