Skip to content

Sync: Add action hook to enqueue_action#8829

Merged
jeherve merged 2 commits intoAutomattic:masterfrom
rebeccahum:vip/add_hook_queue_sync
Feb 19, 2018
Merged

Sync: Add action hook to enqueue_action#8829
jeherve merged 2 commits intoAutomattic:masterfrom
rebeccahum:vip/add_hook_queue_sync

Conversation

@rebeccahum
Copy link
Copy Markdown
Contributor

@rebeccahum rebeccahum commented Feb 13, 2018

Fixes #

Changes proposed in this Pull Request:

  • I'd like to add an action hook to enqueue_action when anything on the whitelist gets sent to the queue to sync.

Testing instructions:

  • Hook onto it via add_action.

Changelog entry

  • We added the jetpack_sync_action_before_enqueue action that's done when anything gets enqueued before being synchronized to WordPress.com servers.

@rebeccahum rebeccahum requested a review from a team as a code owner February 13, 2018 18:20
@jeherve jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Package] Sync [Pri] Low labels Feb 13, 2018
Copy link
Copy Markdown
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.

Just a couple of minor changes.

/**
* Add an action hook to execute when anything on the whitelist gets sent to the queue to sync.
*
* @since 5.8
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since 5.8 has already shipped, could you replace with 5.9.0?

Thank you!

* Add an action hook to execute when anything on the whitelist gets sent to the queue to sync.
*
* @since 5.8
*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add @module sync here? We use this to create categories in our codex:
https://developer.jetpack.com/module/sync/

*
* @param array The action parameters
*/
do_action( "jetpack_action_before_enqueue" );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you could use single quotes here. And I would suggest renaming the action to jetpack_sync_action_before_enqueue, so it's a bit more descriptive.

*
* @since 5.8
*
* @param array The action parameters
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you add a parameter here, you will need to pass one to the hook as well.

@jeherve jeherve 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 Feb 13, 2018
@rebeccahum
Copy link
Copy Markdown
Contributor Author

@jeherve Thanks! Changes complete.

@jeherve jeherve 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 15, 2018
Copy link
Copy Markdown
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.

LGTM!

@jeherve jeherve 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 15, 2018
@oskosk oskosk added this to the 5.9 milestone Feb 19, 2018
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

@jeherve jeherve merged commit 0df4dc0 into Automattic:master Feb 19, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@rebeccahum rebeccahum deleted the vip/add_hook_queue_sync branch March 12, 2018 12:51
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Package] Sync [Pri] Low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants