Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Download required WP Core asset dependency files #310

Merged
merged 3 commits into from
Jan 7, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Jan 6, 2020

As of WordPress/wordpress-develop@a35e46a, an asset file is now generated for each WordPress JS dependency in Core. This creates a problem when running PHPUnit tests on Travis as these files are not generated during the WordPress setup process (as can be seen here for example).

This PR resolves this issue by downloading the wp-includes/assets/dist folder from WordPress' Core SVN repo during the WordPress installation.

@pierlon pierlon requested a review from kasparsd January 6, 2020 19:51
Copy link
Contributor

@kasparsd kasparsd left a comment

Choose a reason for hiding this comment

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

Thanks for opening the pull request @pierlon! I've noticed this issue on some other project repos, too.

What do you think about using the full wp-includes directory from the release repo? I think we're using the source repo mainly to get the phpunit related helpers.

@@ -585,6 +585,10 @@ function install_wp {
mkdir -p "$WP_CORE_DIR/src/wp-includes/css/dist/block-library"
touch "$WP_CORE_DIR/src/wp-includes/css/dist/block-library/style.css"

# Download required asset dependency files
local SVN_CORE_URL=${SVN_URL/develop/core}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the full wp-includes directory from the release repository?

https://core.svn.wordpress.org/trunk/wp-includes

That would solve both the block-library issue above and this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure that makes sense.

Copy link
Contributor

@kasparsd kasparsd left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @pierlon!

@kasparsd kasparsd merged commit d499ff7 into master Jan 7, 2020
@kasparsd kasparsd deleted the fix/wp-core-asset-deps branch January 7, 2020 15:14
pierlon added a commit to ampproject/amp-wp that referenced this pull request Jan 7, 2020
@swissspidy
Copy link

Is this change worth it? Core has just fixed this for trunk: https://core.trac.wordpress.org/changeset/47048

@kasparsd
Copy link
Contributor

kasparsd commented Jan 7, 2020

@swissspidy Oh, great -- that's a better fix!

This fix has been released as 1.3.1 already. Do we still need to keep it for this fix?

@pierlon
Copy link
Contributor Author

pierlon commented Jan 7, 2020

The change would resolve the block-library issue as stated in #310 (comment), so I think it's OK.

@pierlon
Copy link
Contributor Author

pierlon commented Jan 7, 2020

This would also prevent issues such as ampproject/amp-wp#3847 from ever happening again.

westonruter pushed a commit to ampproject/amp-wp that referenced this pull request Jan 8, 2020
* Update wp-dev-lib package to fix Travis errors

See: xwp/wp-dev-lib#310

* Update composer.lock

* Make phpcs happy

* Revert "Pull the built `block-libray` package from Gutenberg SVN if it does not exists (#3847)"

This reverts commit 12c1d47

* Revert update of indirect packages

* Update wp-dev-lib to 1.3.2
westonruter pushed a commit to ampproject/amp-wp that referenced this pull request Jan 8, 2020
* Update wp-dev-lib package to fix Travis errors

See: xwp/wp-dev-lib#310

* Update composer.lock

* Make phpcs happy

* Revert "Pull the built `block-libray` package from Gutenberg SVN if it does not exists (#3847)"

This reverts commit 12c1d47

* Revert update of indirect packages

* Update wp-dev-lib to 1.3.2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants