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

Use spread operator instead of func_get_args #13699

Merged
merged 4 commits into from
Oct 21, 2019
Merged

Use spread operator instead of func_get_args #13699

merged 4 commits into from
Oct 21, 2019

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Oct 9, 2019 •
edited
Loading

PHP 5.6 supports the spread operator (...$var) for variadic functions. As an language construct, it is faster than using func_get_args. As a minor tweak, this PR upgrades instances where the function had no declared arguments and fully depended on func_get_args when it wanted to process variadic arguments.

Intentionally not including what is already pending in #13695

Changes proposed in this Pull Request:

  • Replace instances of func_get_args with the spread operator.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • n/a

Testing instructions:

  • Run unit tests (some changes involved the unit testing structure for JITM)
  • Interact with WP.com and ensure everything works as expected (XMLRPC)
  • Add posts, change options, etc, and verify those changes synced to WP.com.

Proposed changelog entry for your changes:

  • Performance: Modernize various parts of the code to use PHP 5.6+ functionality.

@kraftbj kraftbj added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Focus] Performance [Status] In Progress labels Oct 9, 2019
@kraftbj kraftbj requested a review from a team as a code owner October 9, 2019 21:27
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33811-code before merging this PR. Thank you!

@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 Oct 9, 2019
@jetpackbot
Copy link

jetpackbot commented Oct 9, 2019 •
edited
Loading

Warnings
⚠️

pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 82ecf1a

@kraftbj
Copy link
Contributor Author

kraftbj commented Oct 9, 2019

Reverted the changes to the IXR classes since the extended class from Core is still using func_get_args. See https://core.trac.wordpress.org/ticket/48267

@kraftbj kraftbj mentioned this pull request Oct 10, 2019
Closed
11 tasks
@kraftbj kraftbj added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Oct 15, 2019
@kraftbj kraftbj added this to the 7.9 milestone Oct 17, 2019
@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 Oct 21, 2019
@kraftbj kraftbj changed the title Try: Use spread operator instead of func_get_args Use spread operator instead of func_get_args Oct 21, 2019
@kraftbj kraftbj merged commit 1e240a5 into master Oct 21, 2019
@kraftbj kraftbj deleted the upgrade/spread-operator branch October 21, 2019 15:42
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 21, 2019
@kraftbj
Copy link
Contributor Author

kraftbj commented Oct 21, 2019

r198195-wpcom

@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Oct 23, 2019
jeherve added a commit that referenced this pull request Oct 23, 2019
@jeherve
Changelog: add #13699
64efb4e
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
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

@kraftbj kraftbj

Labels
[Focus] Performance Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
7.9
Development

Successfully merging this pull request may close these issues.
4 participants
@kraftbj @matticbot @jetpackbot @jeherve