-
Notifications
You must be signed in to change notification settings - Fork 812
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 jetpack-composer-plugin
to wpcomsh
and mu-wpcom-plugin
#41185
Add jetpack-composer-plugin
to wpcomsh
and mu-wpcom-plugin
#41185
Conversation
The `wp i18n` command ignores files under `vendor/`, on the assumption that Composer packages probably aren't using WordPress's i18n functions. This doesn't hold true for many of our packages, such as `jetpack-mu-wpcom`. Our workaround for that is a Composer plugin that installs "jetpack-library" packages under `jetpack_vendor/` instead, which `wp i18n` does not ignore. See Problem 5 at https://developer.wordpress.com/2022/01/06/wordpress-plugin-i18n-webpack-and-composer/ for details. Presumably we don't need to worry about Problem 6 for these plugins, as they're only used on WordPress.com and we'd presumably have heard about it already if that problem wasn't worked around somehow at the platform level there.
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Looks like deploying this one to Simple may be a pain. I'll probably want to add in the following:
|
…-plugin-to-wpcomsh-and-mu-wpcom-plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a reasonable iteration toward the goal. I'm not sure I understand the failed WP.com tests, though...e.g. why would linting be complaining?
All the wpcom test failures are because of files like require_once JETPACK_MU_WPCOM_PLUGIN_LOADER_PATH . '/vendor/automattic/jetpack-masterbar/src/wp-posts-list/bootstrap.php'; which will need to be changed to |
) The `wp i18n` command ignores files under `vendor/`, on the assumption that Composer packages probably aren't using WordPress's i18n functions. This doesn't hold true for many of our packages, such as `jetpack-mu-wpcom`. Our workaround for that is a Composer plugin that installs "jetpack-library" packages under `jetpack_vendor/` instead, which `wp i18n` does not ignore. See Problem 5 at https://developer.wordpress.com/2022/01/06/wordpress-plugin-i18n-webpack-and-composer/ for details. Presumably we don't need to worry about Problem 6 for these plugins, as they're only used on WordPress.com and we'd presumably have heard about it already if that problem wasn't worked around somehow at the platform level there.
) The `wp i18n` command ignores files under `vendor/`, on the assumption that Composer packages probably aren't using WordPress's i18n functions. This doesn't hold true for many of our packages, such as `jetpack-mu-wpcom`. Our workaround for that is a Composer plugin that installs "jetpack-library" packages under `jetpack_vendor/` instead, which `wp i18n` does not ignore. See Problem 5 at https://developer.wordpress.com/2022/01/06/wordpress-plugin-i18n-webpack-and-composer/ for details. Presumably we don't need to worry about Problem 6 for these plugins, as they're only used on WordPress.com and we'd presumably have heard about it already if that problem wasn't worked around somehow at the platform level there.
) The `wp i18n` command ignores files under `vendor/`, on the assumption that Composer packages probably aren't using WordPress's i18n functions. This doesn't hold true for many of our packages, such as `jetpack-mu-wpcom`. Our workaround for that is a Composer plugin that installs "jetpack-library" packages under `jetpack_vendor/` instead, which `wp i18n` does not ignore. See Problem 5 at https://developer.wordpress.com/2022/01/06/wordpress-plugin-i18n-webpack-and-composer/ for details. Presumably we don't need to worry about Problem 6 for these plugins, as they're only used on WordPress.com and we'd presumably have heard about it already if that problem wasn't worked around somehow at the platform level there.
A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. #41185
* Fix: Ensure correct MD5 hash lookup for script translation files A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. #41185
* Fix: Ensure correct MD5 hash lookup for script translation files A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. Automattic/jetpack#41185 Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/13396644819 Upstream-Ref: Automattic/jetpack@e8cf9c2
* Fix: Ensure correct MD5 hash lookup for script translation files A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. Automattic/jetpack#41185 Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/13396644819 Upstream-Ref: Automattic/jetpack@e8cf9c2
* Fix: Ensure correct MD5 hash lookup for script translation files A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. Automattic/jetpack#41185 Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/13396644819 Upstream-Ref: Automattic/jetpack@e8cf9c2
* Fix: Ensure correct MD5 hash lookup for script translation files A recent change renamed `vendor/` to `jetpack_vendor/`, causing mismatches in generated MD5 hashes and resulting in missing translation JSON files. Automattic/jetpack#41185 Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/13396644819 Upstream-Ref: Automattic/jetpack@e8cf9c2
Proposed changes:
The
wp i18n
command ignores files undervendor/
, on the assumption that Composer packages probably aren't using WordPress's i18n functions. This doesn't hold true for many of our packages, such asjetpack-mu-wpcom
.Our workaround for that is a Composer plugin that installs "jetpack-library" packages under
jetpack_vendor/
instead, whichwp i18n
does not ignore.See Problem 5 at
https://developer.wordpress.com/2022/01/06/wordpress-plugin-i18n-webpack-and-composer/ for details.
Presumably we don't need to worry about Problem 6 for these plugins, as they're only used on WordPress.com and we'd presumably have heard about it already if that problem wasn't worked around somehow at the platform level there.
Other information:
Jetpack product discussion
p1737162933817909-slack-C05Q5HSS013
Related to p58i-jdp-p2 and 170251-ghe-Automattic/wpcom
Does this pull request change what data or activity we track or use?
No
Testing instructions:
vendor/
tojetpack_vendor/
.