Skip to content

Conversation

@tyxla
Copy link
Member

@tyxla tyxla commented Aug 21, 2019

As suggested by @mdawaffe, in #13256 (comment) the logic we introduced in #13253 for splitting the remote media upload handler registration and the jetpack auth didn't make much sense. This follows his advice to unify the handler registration logic, and make it clear that the Jetpack auth is needed specifically by the remote file upload requests.

Changes proposed in this Pull Request:

  • Media: Unify remote file upload handler registration logic

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

Testing instructions:

  • Checkout this branch on a connected Jetpack site.
  • Try uploading a file via Calypso and make sure it works correctly.

Proposed changelog entry for your changes:

  • Media: Unify remote file upload handler registration logic

@tyxla tyxla added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it General [Status] Needs Review This PR is ready for review. [Pri] Normal [Focus] Jetpack DNA labels Aug 21, 2019
@tyxla tyxla added this to the 7.7 milestone Aug 21, 2019
@tyxla tyxla requested review from a team and mdawaffe August 21, 2019 11:02
@tyxla tyxla self-assigned this Aug 21, 2019
@jetpackbot
Copy link
Collaborator

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: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against 5fff815

Copy link
Member

@mdawaffe mdawaffe left a comment

Choose a reason for hiding this comment

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

LGTM ;) Thanks!

I could see the file upload feature being its own package some day, but there's no need for it now.

@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 Aug 22, 2019
@tyxla tyxla merged commit 08d6fec into master Aug 22, 2019
@tyxla tyxla deleted the update/unify-remote-request-handler-registration-logic branch August 22, 2019 09:42
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Focus] Jetpack DNA General [Pri] Normal [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.

6 participants