Skip to content

Conversation

@jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Jul 16, 2018

After updating the Babel cache key creator to use @babel/core, the compatibility bridge is not used by anything else and can be removed.

Edit: repurposed to moving the bridge to devDependencies as it's used only by Jest now.

@jsnajdr jsnajdr added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Build [Type] Janitorial labels Jul 16, 2018
@jsnajdr jsnajdr self-assigned this Jul 16, 2018
@matticbot
Copy link
Contributor

@jsnajdr jsnajdr requested a review from a team July 16, 2018 14:07
@jsnajdr jsnajdr force-pushed the remove/babel-core-bridge branch from 07b326e to 32ab9de Compare July 16, 2018 14:09
Copy link
Contributor

@blowery blowery left a comment

Choose a reason for hiding this comment

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

LGTM assuming tests pass

@jsnajdr
Copy link
Member Author

jsnajdr commented Jul 16, 2018

Hmm, it turns out it's required by babel-jest. Can't remove it, but can move it to devDependencies, too :)

After updating the Babel cache key creator to use `@babel/core`, the compatibility bridge
is used only by Jest and therefore can be a dev dependency.
@jsnajdr jsnajdr force-pushed the remove/babel-core-bridge branch from 32ab9de to c6e804a Compare July 16, 2018 14:30
@jsnajdr jsnajdr changed the title Remove the babel-core bridge package Move the babel-core bridge package to devDependencies Jul 16, 2018
@blowery
Copy link
Contributor

blowery commented Jul 16, 2018

ah yes jestjs/jest#4918

@blowery
Copy link
Contributor

blowery commented Jul 16, 2018

still approved :)

@jsnajdr jsnajdr merged commit 1b039c3 into master Jul 17, 2018
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 17, 2018
@jsnajdr jsnajdr deleted the remove/babel-core-bridge branch July 17, 2018 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants