Skip to content

Conversation

@mdawaffe
Copy link
Member

@mdawaffe mdawaffe commented Jun 4, 2019

Fixes #12538

Changes proposed in this Pull Request:

  • Moves class.jetpack-xmlrpc-server.php to packages/xmlrpc-server.

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

N/A

Testing instructions:

  • phpunit --filter=WP_Test_Jetpack_XMLRPC_Server

Proposed changelog entry for your changes:

None required.

TODO

  • Move tests to package
  • Resolve dependencies (JetpackTracking, Jetpack_Options, Jetpack, Jetpack_Error, Jetpack_Client_Server, Jetpack_IXR_Client, WP_Error, IXR_Error, Jetpack_Data, WPCOM_JSON_API, …?)

@mdawaffe mdawaffe requested a review from a team June 4, 2019 21:23
@mdawaffe mdawaffe self-assigned this Jun 4, 2019
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jun 4, 2019

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.

Scheduled Jetpack release: July 2, 2019.
Scheduled code freeze: June 25, 2019

Generated by 🚫 dangerJS against 8e0ceb4

@mdawaffe
Copy link
Member Author

mdawaffe commented Jun 4, 2019

I don't think this is the right approach. Rather than a package that provides all the XML-RPC methods, we should have a package that makes it easy to register XML-RPC methods:

namespace ….

class XMLRPC_Server_Methods {
  public function add( string $method_name, array $auth_reqs, callable $callback ) {
    /*
     * $auth_reqs looks something like:
     * [
     *     'public' => (bool), // can be accessed without auth?
     *     'blog'   => (bool), // can be accessed with just a Blog Token?
     *     'user'   => (bool), // requires a User Token?
     * ]
     */
  }
}

Then this package wouldn't have to worry about any dependencies of its own.

@mdawaffe mdawaffe force-pushed the try/xmlrpc-server-package-classmapped branch from 6199c08 to 8e0ceb4 Compare June 7, 2019 23:29
@mdawaffe
Copy link
Member Author

mdawaffe commented Jun 8, 2019

I think we can hold off on this until #12630 settles. It may be this PR isn't needed.

Copy link
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

why not

@lezama lezama merged commit b423a33 into feature/jetpack-packages Jun 17, 2019
lezama added a commit that referenced this pull request Jun 17, 2019
@lezama
Copy link
Contributor

lezama commented Jun 17, 2019

hmm, merged to the wrong branch, @dereksmart @jeherve shouldn't we delete feature/jetpack-packages?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants