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

Add __meta property to BCD build #14129

Merged
merged 11 commits into from
May 18, 2022

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Dec 24, 2021

This PR fixes #7358 by adding a __meta property to the BCD root, containing the build's version number. This allows for easier access of the version number for NodeJS users, and straight-up allows access for Deno/browser/Python/etc. users entirely.

@github-actions github-actions bot added the scripts 📜 Issues or pull requests regarding the scripts in scripts/. label Dec 24, 2021
@queengooborg queengooborg marked this pull request as draft December 24, 2021 08:43
@queengooborg queengooborg marked this pull request as ready for review December 24, 2021 19:08
@foolip
Copy link
Collaborator

foolip commented Apr 21, 2022

This change is not without risk and I think would require at least a minor version bump. I am fairly skeptical of making this change unless there are multiple requests from BCD consumers.

@queengooborg
Copy link
Collaborator Author

I think that we should include this in a major semver bump actually, since it is a potentially breaking change. In doing so, however, this might help our consumers understand that keys starting with __ are special in BCD, such as compatibility info or otherwise.

I think if we are to do this, we should land it around the same time we start releasing ESM builds.

@queengooborg queengooborg added semver-major-bump 🚨 A change that is potentially breaking for consumers needs-release-note 📰 labels Apr 22, 2022
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, but ...

  1. ... wouldn't it make sense to wrap the version in a __meta object (with one key version), so that we can extend this in the future?
  2. ... this is probably a breaking change for consumers that just loop over the data (and this makes me wonder if we have other breaking changes in the pipeline - possibly among your open PRs - that should be bundled with this).

@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@queengooborg queengooborg changed the title Add __version property to BCD build Add __meta property to BCD build May 12, 2022
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@queengooborg
Copy link
Collaborator Author

I'm going to hold off on merging this for the time being since it is a major semver bump. I'd like to get other changes merged alongside this one before the next release.

@caugner
Copy link
Contributor

caugner commented May 16, 2022

@queengooborg Would it make sense to create a v5 branch for this?

@queengooborg
Copy link
Collaborator Author

queengooborg commented May 17, 2022

Maybe so, yeah -- my plan was originally to hold off on a v5 release until #16232 is merged (and maybe the ESM internal upgrades), but also to hold off so we get a v4.2.0 release... :D

@queengooborg queengooborg merged commit b73a46f into mdn:main May 18, 2022
@queengooborg queengooborg deleted the scripts/bcd-version-in-build branch May 18, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scripts 📜 Issues or pull requests regarding the scripts in scripts/. semver-major-bump 🚨 A change that is potentially breaking for consumers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose BCD version in exported object?
3 participants