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.

Sign up for GitHub

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

Jump to bottom

Blocks: Make webpack.config.extensions.js extend calypso-build's #11802

Merged
merged 3 commits into from
Apr 8, 2019
Merged

Blocks: Make webpack.config.extensions.js extend calypso-build's #11802

merged 3 commits into from
Apr 8, 2019

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Apr 1, 2019 •
edited
Loading

Changes proposed in this Pull Request:

  • Bump @automattic/calypso-build to 1.0.0-alpha.2, which includes a basline webpack.config.js.
  • Base webpack.config.extensions.js on that.

Testing instructions:

yarn distclean
yarn build-extensions

Verify that index.json is copied to _inc/blocks/, that there are .deps.json files listing dependencies, and that blocks are still working.

Proposed changelog entry for your changes:

Blocks: Make webpack.config.extensions.js extend @automattic/calypso-build's

@ockham ockham added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Apr 1, 2019
@ockham ockham self-assigned this Apr 1, 2019
@ockham ockham requested a review from a team April 1, 2019 23:21
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello ockham! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D26294-code before merging this PR. Thank you!

@matticbot

This comment has been minimized.

Sign in to view
1 similar comment
@matticbot

This comment has been minimized.

Sign in to view
@jetpackbot

This comment has been minimized.

Sign in to view
@ockham ockham mentioned this pull request Apr 2, 2019
Merged
sirreal
sirreal reviewed Apr 3, 2019
View reviewed changes
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Noting some thoughts we had while on a call.

webpack.config.extensions.js Outdated Show resolved Hide resolved
webpack.config.extensions.js Outdated Show resolved Hide resolved
@sirreal sirreal mentioned this pull request Apr 3, 2019
Closed
@matticbot

This comment has been minimized.

Sign in to view
1 similar comment
@matticbot

This comment has been minimized.

Sign in to view
@ockham ockham force-pushed the update/use-calypso-build-webpack-config branch 2 times, most recently from bc682c3 to 3733e77 Compare April 3, 2019 18:30
@ockham ockham added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Apr 3, 2019
@ockham ockham requested a review from a team April 3, 2019 18:34
@matticbot
Copy link
Contributor

ockham, Your synced wpcom patch D26294-code has been updated.

sirreal
sirreal reviewed Apr 4, 2019
View reviewed changes
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

One note, code looks good, I'll test…

webpack.config.extensions.js Show resolved Hide resolved
sirreal
sirreal previously approved these changes Apr 4, 2019
View reviewed changes
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Seems to work well, I've tested normal and production bundles 👍

@ockham ockham mentioned this pull request Apr 4, 2019
Merged
simison
simison reviewed Apr 5, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
@simison simison mentioned this pull request Apr 5, 2019
Merged
1 task
simison
simison reviewed Apr 5, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
@ockham ockham force-pushed the update/use-calypso-build-webpack-config branch from 3733e77 to 9eb958e Compare April 5, 2019 12:28
@jeherve
Copy link
Member

jeherve commented Apr 5, 2019

This is going to need a rebase I am afraid.

@ockham ockham force-pushed the update/use-calypso-build-webpack-config branch from 9eb958e to cc79f4a Compare April 5, 2019 20:43
@ockham ockham added [Status] Blocked / Hold and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 5, 2019
@ockham
Copy link
Contributor Author

ockham commented Apr 5, 2019

Rebased. I also need to bump calypso-build tho, this will take a few.

@ockham ockham mentioned this pull request Apr 5, 2019
Merged
ockham added a commit to Automattic/wp-calypso that referenced this pull request Apr 5, 2019
@ockham
Packages: Bump calypso-build version to 1.0.0-alpha.3 (#32080)
c6795b7 
Mostly for Automattic/jetpack#11802, which needs the package to contain #31513.
Sticking with alpha versioning for a bit longer since there are a few more changes we know we'd like to get in before considering its interface somewhat stable.
@ockham
Copy link
Contributor Author

ockham commented Apr 5, 2019 •
edited
Loading

Rebased. I also need to bump calypso-build tho, this will take a few.

Done. Should be ready for review again.

@ockham ockham added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Blocked / Hold labels Apr 5, 2019
@simison simison force-pushed the update/use-calypso-build-webpack-config branch from 5e3f8e6 to d164b37 Compare April 8, 2019 08:04
@simison
Copy link
Member

simison commented Apr 8, 2019 •
edited
Loading

Rebased with changes in package.json (specifically calypso-build update here: #11960)

simison
simison approved these changes Apr 8, 2019
View reviewed changes
Copy link
Member

@simison simison left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good and tests well.

Tested with blocks and Jetpack dashboard using yarn build and yarn build-extensions. Json files are there, too. 👍

@ockham
Copy link
Contributor Author

ockham commented Apr 8, 2019

Thanks Osk!

I'll force wpcom status to Green since I need to update a bunch of dependencies on WP.com in a separate phab diff before I can rebase D26294-code. I'm on it, but I don't want to block this PR anymore (since I'm worried I'd otherwise lose Crew approval again).

@ockham ockham merged commit 544518f into master Apr 8, 2019
@ockham ockham deleted the update/use-calypso-build-webpack-config branch April 8, 2019 10:29
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 8, 2019
@lezama
Copy link
Contributor

lezama commented Apr 10, 2019

this is beautiful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirreal sirreal sirreal left review comments

@oskosk oskosk oskosk approved these changes

@simison simison simison approved these changes

Assignees

@ockham ockham

Labels
[Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ockham @matticbot @jetpackbot @jeherve @simison @lezama @oskosk @sirreal @kraftbj