-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Decaffeinate driver #6913
Decaffeinate driver #6913
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
|
Test summaryRun details
View run in Cypress Dashboard ➡️ FailuresThis comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
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.
You've commited all the original.coffee
files - these should just be for reference at the time of conversion - so you have the original to reference while cleaning up. These should not be committed. You need to delete the original.coffee
files before committing any cleanup (See #2 #2690 (comment)).
We should probably have a task for this to make this easier with such a large conversion like this @bkucera?
Also 100% agree about the error_messages file. I tried to convert this once and stopped pretty quickly in 😳 |
more js cleanup cleanup driver tests review a few more test suites outside of test/cypress remove original coffee
1d4cdc5
to
6ca2de5
Compare
Closing this attempt for now. Ran into hard-to-track-down issues with the e2es. The error messages had to do with globals like I think driver may be extra-difficult to move back to JS because it defines how so much of cypress actually works. |
As part of the conversion of the coffeescript code, we convert driver code to JS.
The only exception is
driver/src/cypress/error_messages.coffee
, which after conversion didn't feel maintainable. (@bkucera mentioned it might be good to rewrite that templating system from scratch in JavaScript.)User facing changelog
Additional details
driver
package`)How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?