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

fix: bump the blockly peer dependency in dev-tools and remove version probing #1183

Merged
merged 4 commits into from
Jul 15, 2022

Conversation

BeksOmega
Copy link
Contributor

@BeksOmega BeksOmega commented Jul 14, 2022

Description

Work on #882 Also fix google/blockly#6204

Requires us to bump devtools to only work with v8+.

@BeksOmega BeksOmega requested a review from a team as a code owner July 14, 2022 20:20
@BeksOmega BeksOmega requested review from maribethb and removed request for a team July 14, 2022 20:20
@maribethb
Copy link
Contributor

If this requires us to update the dev-tools dependency then do that in this PR and mark this as a breaking change, then lgtm

@BeksOmega
Copy link
Contributor Author

BeksOmega commented Jul 15, 2022

The id generator fix was already reviewed by Aaron in #1184 No need to re-review. Just moving everything into one PR.

@BeksOmega BeksOmega changed the title fix: use globalThis instead of Blockly...globalThis fix: bump the blockly peer dependency in dev-tools and remove version probing Jul 15, 2022
@BeksOmega BeksOmega merged commit 79ce104 into google:master Jul 15, 2022
@maribethb
Copy link
Contributor

This should be a breaking change. If someone is using blockly v7 then we would not want npm to automatically update them to this version of the plugin since it is no longer compatible. It has to be a major update so that they manually update when needed. You can probably still update the title of this PR to affect lerna's reading of the commit, but I'm not sure.

And since this would be a major version update of dev-tools I think that means we need to manually update all of the other plugins to depend on the new version because lerna won't keep them up to date after a major change is what we learned last time. So we should take extra care after the next samples release (which I won't be here next week)

@BeksOmega
Copy link
Contributor Author

This should be a breaking change.

Yeah :/ whoops.

You can probably still update the title of this PR to affect lerna's reading of the commit, but I'm not sure.

Pretty sure it won't, because lerna will just be looking at the git history :/ Will revert and reapply with the correct conventional commits.

So we should take extra care after the next samples release (which I won't be here next week)

Sounds good, made a note to myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants