Skip to content

Conversation

@pereirinha
Copy link
Contributor

Allows pushing an asset update — either via Re-sync to Cloudinary or the bulk action Push to Cloudinary — that resets the location of the asset in Cloudinary account to the folder set in WordPress.

Copy link
Contributor

@dugajean dugajean left a comment

Choose a reason for hiding this comment

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

Great work! Works as expected. Left a few comments to check out before moving forward. 👍

*
* @return bool
*/
&& ! apply_filters( 'cloudinary_doing_upload', false )
Copy link
Contributor

Choose a reason for hiding this comment

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

One small correction here:

Suggested change
&& ! apply_filters( 'cloudinary_doing_upload', false )
&& apply_filters( 'cloudinary_doing_upload', false )

Otherwise we default to true. Thanks for addressing this 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dugajean

The default should be true. We should almost always resolve to serve the Cloudinary. However, in the case of uploading, we should bail any change.

Copy link
Contributor

@DavidCramer DavidCramer left a comment

Choose a reason for hiding this comment

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

This looks good to me. It's a nice addition.

@pereirinha pereirinha merged commit 7f6c987 into add/original-image-storage Sep 30, 2020
@pereirinha pereirinha deleted the fix/original-image-storage branch October 7, 2020 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants