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

feat(schemas/browser)!: changes releases from object to array #26061

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

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Feb 26, 2025

Summary

Breaking change: Changes the browsers.*.releases property from an object to an array, moving the version number inside the ReleaseStatement.

Test results and supporting details

Related issues

Fixes #21124.

@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. data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. bulk_update An update to a mass amount of data, or scripts/linters related to such changes scripts Issues or pull requests regarding the scripts in scripts/. labels Feb 26, 2025
@caugner caugner added the semver-major-bump A change that is potentially breaking for consumers label Feb 26, 2025
@github-actions github-actions bot added the size:xl [PR only] >1000 LoC changed label Feb 26, 2025
@caugner caugner added this to the BCD 6.0 milestone Feb 26, 2025
Ran `npx tsx scripts/migrations/013-releases-to-array.ts`
@caugner caugner force-pushed the 21124-releases-to-array branch from 38ab64f to 65aee0b Compare February 26, 2025 13:47
@queengooborg
Copy link
Contributor

Honestly, I'd love to try and find an alternate solution if we can. I feel that since consumers can access the order through other means (either comparing version numbers or release dates, depending on their use of Node.js data), I'm cautiously skeptical about changing the data structure this much.

@ddbeck
Copy link
Collaborator

ddbeck commented Feb 27, 2025

@queengooborg In #21124, I put forward some alternatives, which have their own share of difficulties. It'd be helpful if you could read and respond to the inciting issue.

@caugner one thing Philip raised as a possibility on the original issue is tests that assert that "dates are strictly increasing and that the order matches that which compare-versions produces." Would it be too much to ask to add these tests? I think both of these would be a good idea, to make explicit the assumptions that we've held historically and relied on to transform the objects to arrays.

@caugner
Copy link
Contributor Author

caugner commented Feb 27, 2025

assert that "dates are strictly increasing and that the order matches that which compare-versions produces."

@ddbeck If I understand you correctly, then this assumption already doesn't hold for Node.js releases:

We could enforce it for other browsers, though, if that's an acceptable compromise from your side?

@Elchi3 What's your opinion here?

@Elchi3
Copy link
Member

Elchi3 commented Feb 27, 2025

I think that would be a good enforcement and if we need to exclude nodejs from that rule, that's okay, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bulk_update An update to a mass amount of data, or scripts/linters related to such changes data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. 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:xl [PR only] >1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explicitly order browser releases
4 participants