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

Remove nonreal values from the schema #24174

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Aug 20, 2024

This PR removes nonreal (true and null) values from the BCD schema entirely. This follows the removal of all nonreal values throughout BCD, finalizing the process.

Note: this PR will fail until all the remaining nonreal values have been removed.

Fixes openwebdocs/project#206.

@github-actions github-actions bot added schema Isses or pull requests regarding the JSON schema files used in this project. infra Infrastructure issues (npm, GitHub Actions, releases) of this project docs Issues or pull requests regarding the documentation of this project. linter Issues or pull requests regarding the tests / linter of the JSON files. labels Aug 20, 2024
@github-actions github-actions bot added the scripts Issues or pull requests regarding the scripts in scripts/. label Aug 29, 2024
@caugner caugner marked this pull request as draft October 15, 2024 13:25
@danielhjacobs
Copy link
Contributor

Fixes mdn/yari#11257 by making it a non-issue.

@queengooborg queengooborg marked this pull request as ready for review November 28, 2024 11:31
@queengooborg queengooborg added the semver-major-bump A change that is potentially breaking for consumers label Nov 28, 2024
@queengooborg
Copy link
Collaborator Author

This is now ready to go, whoo!

I've also now labeled this as a major semver bump as this is a potentially breaking change. I thought about it a bit and I realized that TypeScript users may suffer compiling errors if their code expects nonreal values, so to play it safe, I feel this should not be a minor or patch release. (Additionally, it's big news too!)

@caugner
Copy link
Contributor

caugner commented Nov 29, 2024

I agree that this would be a breaking change requiring a major bump, but I would like if we could combine it with other breaking changes we may be thinking about.

Edit: Additionally, I'm wondering if it could make sense to keep nonreal values in the schema, but "just" disallow them for the current browsers we have, so that other browsers or engines could be added more easily in the future. Then again, maybe it's sufficient to use version ranges for that case?

@danielhjacobs
Copy link
Contributor

Then again, maybe it's sufficient to use version ranges for that case?

Note that according to #24293 (comment), @queengooborg is working to eliminate ranges too.

@queengooborg
Copy link
Collaborator Author

I think that ranges would be sufficient for any new browsers we add to BCD, as all "true" values can be expressed in ranges already. There wouldn't be a way to express "null", but our closest option would be to set the feature to "false" and correct it later if we find that the browser does support it. With the efforts that have gone into the mdn-bcd-collector, the majority of our data would be automatically maintained and updated for new browsers, so I feel we're already set there!

I am working on replacing all ranges with exact version numbers as well, but that's a slow process so it'll be a while before ranges are removed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues or pull requests regarding the documentation of this project. infra Infrastructure issues (npm, GitHub Actions, releases) of this project linter Issues or pull requests regarding the tests / linter of the JSON files. schema Isses or pull requests regarding the JSON schema files used in this project. scripts Issues or pull requests regarding the scripts in scripts/. semver-major-bump A change that is potentially breaking for consumers size:l [PR only] 101-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace all nonreal values in BCD with version numbers/ranges
3 participants