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.

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

AT: Add jetpack_sync_send_data_query_args #6317

Merged
merged 3 commits into from
Feb 15, 2017

Conversation

artpi
Copy link
Contributor

@artpi artpi commented Feb 7, 2017

Adds a filter for query_args in Jetpack_Sync_Actions::send_data
We need this filter for changing timeout property while fast-Syncing big sites.

This is not a priority, we don't use it now, but our current solution is clunky.

CC @gravityrail @lamosty

@lamosty
Copy link
Contributor

lamosty commented Feb 7, 2017

Nice work @artpi.

Don't forget to add proper filters/actions docblocks like we do on other places throughout the Jetpack codebase. :)

@jeherve jeherve added [Package] Sync [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Feb 8, 2017
@@ -137,6 +137,8 @@ static function send_data( $data, $codec_name, $sent_timestamp, $queue_id, $chec

$query_args['timeout'] = Jetpack_Sync_Settings::is_doing_cron() ? 30 : 15;

$query_args = apply_filters( 'jetpack_sync_send_data_query_args', $query_args );
Copy link
Member

Choose a reason for hiding this comment

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

As @lamosty suggested, you'll need to add a docblock to document that filter and make our parser and documentation happy. Here is an example:

/**
* Fires on every request before default loading sync listener code.
* Return false to not load sync listener code that monitors common
* WP actions to be serialized.
*
* By default this returns true for cron jobs, non-GET-requests, or requests where the
* user is logged-in.
*
* @since 4.2.0
*
* @param bool should we load sync listener code for this request
*/

You can use 4.7.0 for the @since version.

I would also recommend having a more detailed commit message in the future, as mentioned in PCYsg-1lS-p2

Thank you!

@jeherve jeherve added [Status] In Progress [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Pri] High and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 8, 2017
@gravityrail
Copy link
Contributor

@artpi @jeherve - I added the docblock

@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 14, 2017
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.

LGTM! 🚢

@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 fellow Jetpack developers. Label will be renamed soon. labels Feb 14, 2017
@samhotchkiss samhotchkiss merged commit ea6a4d0 into master Feb 15, 2017
@samhotchkiss samhotchkiss deleted the update/at-send_data-filter branch February 15, 2017 04:40
@samhotchkiss samhotchkiss added [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 15, 2017
@jeherve jeherve added [Status] Has Changelog and removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 20, 2017
jeherve added a commit that referenced this pull request Feb 21, 2017
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Sync [Pri] High [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.

5 participants