Skip to content

AT: Add jetpack_sync_send_data_query_args#6317

Merged
samhotchkiss merged 3 commits intomasterfrom
update/at-send_data-filter
Feb 15, 2017
Merged

AT: Add jetpack_sync_send_data_query_args#6317
samhotchkiss merged 3 commits intomasterfrom
update/at-send_data-filter

Conversation

@artpi
Copy link
Copy Markdown
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
Copy Markdown
Contributor

lamosty commented Feb 7, 2017 •
edited
Loading

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 This PR is ready for review. Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Feb 8, 2017
jeherve
jeherve requested changes Feb 8, 2017
View reviewed changes
sync/class.jetpack-sync-actions.php

$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
Copy Markdown
Member

@jeherve jeherve Feb 8, 2017

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:

jetpack/sync/class.jetpack-sync-actions.php

Lines 40 to 51 in 2e275df

/**
* 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 need more details from you. This label will be auto-added until the PR meets all requirements. [Pri] High and removed [Status] Needs Review This PR is ready for review. labels Feb 8, 2017
@gravityrail
Copy link
Copy Markdown
Contributor

gravityrail commented Feb 14, 2017

@artpi @jeherve - I added the docblock

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 14, 2017
jeherve
jeherve approved these changes Feb 14, 2017
View reviewed changes
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 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
@jeherve
Changelog: add #6317
3cf4a5f
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
@jeherve @dereksmart
Jetpack 4.7: Changelog (#6331)
a9adf86 
* 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

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@artpi artpi

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@artpi @lamosty @gravityrail @jeherve @samhotchkiss