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

Allow v0.7 of Imagine Bundle #935

Closed
wants to merge 3 commits into from
Closed

Allow v0.7 of Imagine Bundle #935

wants to merge 3 commits into from

Conversation

trsteel88
Copy link
Contributor

@trsteel88 trsteel88 commented May 14, 2017

php-imagine/Imagine#547

Q A
Branch? 1.0 and 2.0
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
License MIT

@robfrawley robfrawley changed the title v0.7 resolves a ImageMagick BC Break Allow v0.7 of Imagine Bundle May 15, 2017
@robfrawley robfrawley added the State: Reviewing This item is being reviewed to determine if it should be accepted. label May 15, 2017
@robfrawley robfrawley added this to the 1.8.1 milestone May 15, 2017
Copy link
Collaborator

@robfrawley robfrawley left a comment

Choose a reason for hiding this comment

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

None of the test runs on Travis actually pull and test v0.7; we need to ensure the newer version is actually tested before merging this.

https://travis-ci.org/liip/LiipImagineBundle/jobs/232226661#L232

@trsteel88
Copy link
Contributor Author

@robfrawley I've updated the travis config to test against 0.6 and 0.7.

It looks like something has failed that isn't failed to the 0.7 change? Are you able to re-execute the tests on your end?

@trsteel88
Copy link
Contributor Author

trsteel88 commented May 16, 2017

@robfrawley any idea why that 1 test is failing? That wouldn't be from this PR's changes.

https://travis-ci.org/liip/LiipImagineBundle/jobs/232720653

@robfrawley
Copy link
Collaborator

robfrawley commented May 16, 2017

I'll take a closer look at this late tonight over the weekend.

@trsteel88
Copy link
Contributor Author

Thank you.

@robfrawley
Copy link
Collaborator

@trsteel88 Are you aware of any changes to the grayscale handling with the new Imagine library version?

@trsteel88
Copy link
Contributor Author

@robfrawley no I'm not. However, that failed test is on the older version anyway?

It looks like it's only the php5.3 test failing running ~0.6

@robfrawley
Copy link
Collaborator

I re-executed it; let's see how that goes. Otherwise I'll but this PR and perform some local tests to determine what's going on.

@trsteel88
Copy link
Contributor Author

Any luck with your local tests @robfrawley?

@trsteel88
Copy link
Contributor Author

Hey @robfrawley, how can I help to get this progressed?

@robfrawley
Copy link
Collaborator

robfrawley commented Jun 13, 2017

We need to fix the merge conflicts and resolve the build failures, the latter of which I don't have too much time to figure out ATM. If you can make everything green I'd be happy to include it in the 1.8.1 release, which is likely to hit the streets around 2017-07-01.

@robfrawley
Copy link
Collaborator

@trsteel88 Thanks for your PR; an alternate solution to resolve the issue mentioned in this PR has been opened as #958.

@robfrawley robfrawley closed this Jul 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Reviewing This item is being reviewed to determine if it should be accepted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants