-
Notifications
You must be signed in to change notification settings - Fork 2.2k
breaking: add changes needed for release 15 #983
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
Conversation
✅ Deploy Preview for cypress-example-kitchensink ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cypress-example-kitchensink
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress-example-kitchensink
|
| Branch Review |
release/15.0.0
|
| Run status |
|
| Run duration | 05m 42s |
| Commit |
|
| Committer | Bill Glesias |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
123
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
29.29%
|
|
|---|---|
|
|
27
|
|
|
35
|
Accessibility
91.22%
|
|
|---|---|
|
|
5 critical
2 serious
4 moderate
0 minor
|
|
|
217
|
cypress-example-kitchensink
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress-example-kitchensink
|
| Branch Review |
release/15.0.0
|
| Run status |
|
| Run duration | 05m 29s |
| Commit |
|
| Committer | Bill Glesias |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
123
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
29.29%
|
|
|---|---|
|
|
27
|
|
|
35
|
Accessibility
90.95%
|
|
|---|---|
|
|
5 critical
2 serious
4 moderate
0 minor
|
|
|
221
|
|
This looks like it would be a breaking change, needing a new major version, or am I jumping the gun? |
|
@MikeMcC399 it requires Cypress 15. We don't really have
@MikeMcC399 correct this would be for Cypress 15. I'm still updating the PR |
|
We're on the same page! This repo needs The CONTRIBUTING document is very weak on details. 🙁 See PR #967 for an example. |
ah so I will likely need to rebase merge this in. Message will need to be something like: or something similar? This PR will also fail until we update it with a 15 binary. It's a bit of a chicken or the egg problem since we test the kitchensink in the app repo as well |
|
The commit details you suggest look good. Have you considered putting in conditional logic based on the version of Cypress ? If that works, it might not need to be a breaking change Edit: I followed my own suggestions. See comment following ... |
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 PR has a number of shortcomings which I hope that I have overcome in the alternative proposed PR #984
- Change not backwards compatible with Cypress 14
- PR can't be merged until Cypress 15 available
- PR is a breaking change
The repo is configured with Cypress 14 and the change is not compatible with this version. I added conditional logic based on using Cypress.version. This allows tests to be run successfully on Cypress 14 and it means that the PR can be merged without waiting for Cypress 15 to be released. It also means that after merging a new minor version of cypress-example-kitchensink (probably v5.1.0 unless another PR gets merged in between), not a breaking change with a new major version, and this can then immediately be added to the develop branch of https://github.com/cypress-io/cypress before Cypress 15 is released.
- Source code for https://example.cypress.io/commands/misc not updated
This issue is also resolved in the replacement PR
So my suggestion is to review PR #984, and if it is a suitable replacement for this PR, then merge the alternate one and close this one.
|
@MikeMcC399 we can just close this in favor of #984 |
No description provided.