Skip to content
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

Jump to bottom

fix: make Cypress.ConfigOptions follow json Schema #17251

Merged
merged 2 commits into from Jul 12, 2021
Merged

fix: make Cypress.ConfigOptions follow json Schema #17251

merged 2 commits into from Jul 12, 2021

Conversation

elevatebart
Copy link
Contributor

User facing changelog

The Cypress.ConfigOptions, Cypress.ResolvedConfigOptions and Cypress.RuntimeConfigOption types have been updated so that ConfigOptions match the json schema

Additional details

This fixes inconsistencies in cypress externally exposed types

@elevatebart elevatebart requested a review from a team as a code owner July 8, 2021 15:11
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 8, 2021

Thanks for taking the time to open a PR!

@elevatebart elevatebart requested review from flotwig and kuceb and removed request for a team July 8, 2021 15:11
@cypress
Copy link

cypress bot commented Jul 8, 2021 •
edited


Test summary

4078 • 0 • 53 • 1 • Flakiness 0

Run details
Project cypress
Status Passed
Commit 6feb5ab
Started Jul 9, 2021 1:38 PM
Ended Jul 9, 2021 1:49 PM
Duration 10:54 💡
OS Linux Debian - 10.8
Browser Firefox 88

View run in Cypress Dashboard ➡️

This 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

@elevatebart
fix: make Cypress.ConfigOptions follow json Schema
b43c415 
BREAKING CHANGE: `Cypress.ConfigOptions`, `Cypress.ResolvedConfigOptions`
and `Cypress.RuntimeConfigOptions` have been modified.
If you are using them please check your types
@jennifer-shehane jennifer-shehane added the type: breaking change Requires a new major release version label Jul 8, 2021
@jennifer-shehane jennifer-shehane mentioned this pull request Jul 12, 2021
Merged
7 tasks
@flotwig
Copy link
Contributor

flotwig commented Jul 12, 2021

type: breaking change

@elevatebart I don't see any test updates, can you clarify for me how this is a breaking change?

@elevatebart
Copy link
Contributor Author

@flotwig
We never tested out types, but they were still exposed.
A developer could have used them in a plugin or another community tool to help type their inputs.

It is very unlikely though, since the types were wrong and nobody complained.
I can remove the "breaking change" if you prefer.

flotwig
flotwig approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. I see how this could be a breaking change even tho it's a typefix, I guess it's worth documenting in the CL as such too.

@flotwig
Copy link
Contributor

flotwig commented Jul 12, 2021

@elevatebart gonna merge this, but it would be extra sweet if you could add tests that help keep these types, server/lib/config, and cypress.schema.json in sync.

@flotwig flotwig merged commit 16a4d91 into 8.0-release Jul 12, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 20, 2021

Released in 8.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v8.0.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jul 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@flotwig flotwig flotwig approved these changes

@kuceb kuceb Awaiting requested review from kuceb

Assignees
No one assigned
Labels
type: breaking change Requires a new major release version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@elevatebart @flotwig @jennifer-shehane