Skip to content

Conversation

@kbrown9
Copy link
Member

@kbrown9 kbrown9 commented Jan 22, 2021

Changes proposed in this Pull Request:

The Sync package currently calls Jetpack::get_stat_data() to obtain the heartbeat stats array. The Sync package needs to be independent of Jetpack, so use the Heartbeat::generate_stats_array() method instead. When the Jetpack plugin is installed, this method returns the same data as Jetpack::get_stat_data().

I also fixed some phpcs warnings in test_class.jetpack-sync-modules-stats.php.

Jetpack product discussion

  • n/a

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

  • no

Testing instructions:

  1. Install, activate, and connect Jetpack.

  2. Set up your sandbox and Unagi so you can watch sync actions. For more info: PCYsg-lgg-p2

  3. SSH in to your remote site and clear the last_heartbeat Jetpack option using wp cli:

    wp jetpack options delete last_heartbeat

  4. Trigger the heartbeat cron job using wp cli:

    wp cron event run jetpack_v2_heartbeat

  5. You should see the jetpack_sync_heartbeat_stats sync action in Unagi. Expand the meta data and verify that the data includes an array with these elements (version, wp-version, php-version, etc.).

Proposed changelog entry for your changes:

  • n/a. I don't think these changes need a changelog entry.

@kbrown9 kbrown9 added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Package] Sync [Pri] Low [Focus] Jetpack DNA labels Jan 22, 2021
@kbrown9 kbrown9 self-assigned this Jan 22, 2021
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jan 22, 2021
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jan 22, 2021

Scheduled Jetpack release: February 16, 2021.
Scheduled code freeze: February 8, 2021

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 84a483c

The Sync package calls Jetpack::get_stat_data to obtain the heartbeat
stats array. The Sync package needs to be independent of Jetpack, so
use the Heartbeat::generate_stats_array() method instead. When the Jetpack
plugin is installed, this method returns the same data as
Jetpack::get_stat_data.
@kbrown9 kbrown9 force-pushed the update/sync_use_heartbeat branch from 9b1dc3c to 7f0452a Compare January 22, 2021 21:26
@kbrown9 kbrown9 added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jan 23, 2021
@kraftbj kraftbj added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jan 29, 2021
 * Add tests that do not require Jetpack data. These tests set their own hook on the
   'jetpack_heartbeat_stats_array' filter.
 * Add comments to the Jetpack integration tests.
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Feb 11, 2021
@kbrown9 kbrown9 added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 11, 2021
@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 25, 2021
@github-actions
Copy link
Contributor

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.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Include a changelog entry for any meaningful change.
  • ✅ Specify whether this PR includes any changes to data or privacy.

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 🤖


If you are an automattician, once your PR is ready for review add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.undefined


jetpack plugin:

  • Next scheduled release: March 2, 2021.
  • Scheduled code freeze: February 22, 2021

@kbrown9 kbrown9 added this to the 9.6 milestone Feb 25, 2021
@kraftbj kraftbj merged commit 7a208dc into master Feb 25, 2021
@kraftbj kraftbj deleted the update/sync_use_heartbeat branch February 25, 2021 23:13
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Focus] Jetpack DNA [Package] Sync [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low [Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants