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.

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

Add `media fix-orientation` command #108

Merged
merged 10 commits into from Aug 13, 2019

Conversation

@thrijith
Copy link
Member

commented Apr 30, 2019

thrijith added some commits Apr 29, 2019

@thrijith thrijith requested a review from wp-cli/committers as a code owner Apr 30, 2019

thrijith added some commits Apr 30, 2019

Update Scenario description
Require exif extension
@thrijith

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

Unable to understand the reason for test failure here, if exif was not available it should have resulted in error at early stage itself.

The images here need a fix but still not showing in dry-run 😕

@schlessera

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

I wanted to check this on my local machine but I don't have a PHP runtime that works with WP 3.7.

Let's just skip the tests for those old WP versions. What is the earliest where it makes sense to test this?

@thrijith

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

What is the earliest where it makes sense to test this?

Not sure, Could we test for a minimum of 4.0 and see if it works?

@schlessera

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

Yes, let's try that.

@thrijith

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

Yes, let's try that.

Now it has more failures than before 🤦‍♂ latest build, older build.

@schlessera

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

It looks like the problem is just the order in which the files are processed vs the order in which you have the test output to compare to.

Maybe change the tests from:

    Then STDOUT should be:
    """
    1/3 "Portrait Four" (ID {PORTRAIT_FOUR}) will be affected.
    2/3 "Landscape Five" (ID {LANDSCAPE_FIVE}) will be affected.
    3/3 "Landscape Two" (ID {LANDSCAPE_TWO}) will be affected.
    Success: 3 of 3 images will be affected.
    """

to this:

    Then STDOUT should contain:
    """
    3 "Portrait Four" (ID {PORTRAIT_FOUR}) will be affected.
    """

    And STDOUT should contain:
    """
    3 "Landscape Five" (ID {LANDSCAPE_FIVE}) will be affected.
    """

    And STDOUT should contain:
    """
    3 "Landscape Two" (ID {LANDSCAPE_TWO}) will be affected.
    """

    And STDOUT should contain:
    """
    Success: 3 of 3 images will be affected.
    """

thrijith added some commits Aug 12, 2019

thrijith and others added some commits Aug 12, 2019

@schlessera schlessera added this to the 2.0.4 milestone Aug 13, 2019

@schlessera

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

@thrijith I did small changes to the flip/rotate logic. If the tests now pass, I'll merge and tag a new release of this command. If you should notice something, we can still create anther PR to fix, but I want to get going with the release and include this if possible.

@thrijith

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

Sure @schlessera, I will verify the changes once it's tagged, will send a PR in case it needs change.

You can use images in this repo to test the fix, it covers all the cases.

@schlessera schlessera merged commit fff4dc8 into wp-cli:master Aug 13, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@schlessera schlessera changed the title Add `wp media fix-orientation` command Add `media fix-orientation` command Aug 13, 2019

@thrijith thrijith deleted the thrijith:orientation-fix branch Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.