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

packages: Add a bunch of missing deps #37141

Merged
merged 3 commits into from
May 1, 2024
Merged

packages: Add a bunch of missing deps #37141

merged 3 commits into from
May 1, 2024

Conversation

anomiex
Copy link
Contributor

@anomiex anomiex commented Apr 30, 2024

Proposed changes:

The following dependencies are added:

  • packages/backup: packages/my-jetpack
  • packages/boost-core: packages/connection
  • packages/forms: packages/logo, packages/status, packages/sync
  • packages/import: packages/sync
  • packages/search: packages/sync
  • packages/videopress: packages/my-jetpack
  • packages/yoast-promo: packages/assets

Also, the following are added as dev-dependencies and flagged as test-only so they won't result in dependency cycles:

  • packages/config: 14 different things it can conditionally load
  • packages/connection: packages/licensing, packages/sync
  • packages/my-jetpack: packages/search, and mark packages/videopress as test-only too
  • packages/status: packages/connection, packages/identity-crisis, packages/plans
  • packages/sync: packages/search, packages/waf

Note I haven't fully checked that all these really are test-only with any non-test uses properly checked with class_exists or the like. I do note that status→connection is only because of the Jetpack_Options class and is not checked, which seems unfortunate and probably should be fixed somehow at some point.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

None

Does this pull request change what data or activity we track or use?

No

Testing instructions:

  • CI happy?
  • We should probably also check what this does to the build artifacts, were any plugins missing any of these or did they all already have them?

@anomiex anomiex added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Pri] Normal labels Apr 30, 2024
@anomiex anomiex requested a review from a team April 30, 2024 20:35
@anomiex anomiex self-assigned this Apr 30, 2024
@anomiex anomiex mentioned this pull request Apr 30, 2024
Merged
3 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 30, 2024 •
edited
Loading

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the add/lots-of-missing-deps branch.

    • For jetpack-mu-wpcom changes, also add define( 'JETPACK_MU_WPCOM_LOAD_VIA_BETA_PLUGIN', true ); to your wp-config.php file.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack add/lots-of-missing-deps

    bin/jetpack-downloader test jetpack-mu-wpcom-plugin add/lots-of-missing-deps

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added [Feature] Contact Form [Package] Backup [Package] Boost Core [Package] Config [Package] Connection [Package] Forms [Package] Import [Package] My Jetpack [Package] Search Contains core Search functionality for Jetpack and Search plugins [Package] Status [Package] Sync [Package] VideoPress [Package] Yoast Promo [Plugin] Automattic For Agencies Client [Plugin] Backup A plugin that allows users to save every change and get back online quickly with one-click restores. [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] Inspect [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Migration [Plugin] mu wpcom jetpack-mu-wpcom plugin [Plugin] Protect A plugin with features to protect a site: brute force protection, security scanning, and a WAF. [Plugin] Search A plugin to add an instant search modal to your site to help visitors find content faster. [Plugin] Social Issues about the Jetpack Social plugin labels Apr 30, 2024
@github-actions github-actions bot added [Plugin] Starter Plugin [Plugin] VideoPress A standalone plugin to add high-quality VideoPress videos to your site. labels Apr 30, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 30, 2024 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

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.

Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen daily.
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly. The next release is scheduled for May 7, 2024 (scheduled code freeze on May 6, 2024).

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Backup plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Boost plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Search plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Social plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Starter Plugin plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Protect plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Videopress plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Migration plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Mu Wpcom plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Inspect plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Automattic For agencies client plugin:

  • Next scheduled release: June 4, 2024.
  • Scheduled code freeze: May 27, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

jeherve
jeherve previously approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Tests look good, let's give this a try. Did you find all those misses via Phan?

@anomiex
Copy link
Contributor Author

anomiex commented May 1, 2024

Yes, this is all the result of looking at where Phan complained about undeclared classes.

Base automatically changed from add/cli-dependencies-test-only to trunk May 1, 2024 15:15
@anomiex anomiex dismissed jeherve’s stale review May 1, 2024 15:15

The base branch was changed.

@anomiex
packages: Add a bunch of missing deps
158f025 
The following dependencies are added:

* packages/backup: packages/my-jetpack
* packages/boost-core: packages/connection
* packages/forms: packages/logo, packages/status, packages/sync
* packages/import: packages/sync
* packages/search: packages/sync
* packages/videopress: packages/my-jetpack
* packages/yoast-promo: packages/assets

Also, the following are added as dev-dependencies and flagged as
test-only so they won't result in dependency cycles:

* packages/config: 14 different things it can conditionally load
* packages/connection: packages/licensing, packages/sync
* packages/my-jetpack: packages/search, and mark packages/videopress as test-only too
* packages/status: packages/connection, packages/identity-crisis, packages/plans
* packages/sync: packages/search, packages/waf

Note I haven't fully checked that all these really are test-only with
any non-test uses properly checked with `class_exists` or the like. I do
note that status→connection is only because of the `Jetpack_Options`
class and is _not_ checked, which seems unfortunate and probably should
be fixed somehow at some point.
@anomiex
connection: Disable sync in tests, now that it exists
b9731f6
@anomiex
status: Fix tests by alias-mocking Identity_Crisis
bb03b6a
jeherve
jeherve reviewed May 1, 2024
View reviewed changes
projects/packages/connection/composer.json
"automattic/jetpack-changelogger": "@dev"
"automattic/jetpack-changelogger": "@dev",
"automattic/jetpack-licensing": "@dev",
"automattic/jetpack-sync": "@dev"
Copy link
Member

Choose a reason for hiding this comment

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

Is it ok that the Connection package requires sync, while the Sync package requires the connection package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be ok.

  • Composer doesn't care as long as it can come up with a consistent set of packages, which it can.
  • Our build scripts are ok with it now as long as at least one of the deps is marked as "test-only", thanks to cli: Allow flagging deps as test-only #37140.
  • Is there anything else you're concerned about?

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels May 1, 2024
@anomiex anomiex merged commit ec16c44 into trunk May 1, 2024
71 of 73 checks passed
@anomiex anomiex deleted the add/lots-of-missing-deps branch May 1, 2024 16:57
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 1, 2024
@nateweller nateweller mentioned this pull request Jul 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@anomiex anomiex

Labels
Docs [Feature] Contact Form [JS Package] I18n Check Webpack Plugin [Package] Backup [Package] Boost Core [Package] Config [Package] Connection [Package] Forms [Package] Import [Package] My Jetpack [Package] Search Contains core Search functionality for Jetpack and Search plugins [Package] Status [Package] Sync [Package] VideoPress [Package] Yoast Promo [Plugin] Automattic For Agencies Client [Plugin] Backup A plugin that allows users to save every change and get back online quickly with one-click restores. [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] Inspect [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Migration [Plugin] mu wpcom jetpack-mu-wpcom plugin [Plugin] Protect A plugin with features to protect a site: brute force protection, security scanning, and a WAF. [Plugin] Search A plugin to add an instant search modal to your site to help visitors find content faster. [Plugin] Social Issues about the Jetpack Social plugin [Plugin] Starter Plugin [Plugin] VideoPress A standalone plugin to add high-quality VideoPress videos to your site. [Pri] Normal RNA [Tests] Includes Tests [Tools] Development CLI The tools/cli to assist during JP development. [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anomiex @jeherve