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

Added php7.4 #128

Merged
merged 1 commit into from Dec 12, 2019
Merged

Added php7.4 #128

merged 1 commit into from Dec 12, 2019

Conversation

@DonCallisto
Copy link
Contributor

DonCallisto commented Nov 24, 2019

With php7.4 behind the corner, I tought it was good to try this against new version.

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from e521005 to 54a697e Nov 24, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 24, 2019

It seems we'll need to wait for that phpspec/phpspec#1269 to be merged.
Do you prefer to keep this PR open or ... ?
Moreover we can also bump PHPSpec version to 6 (I've seen it won't add nothing useful for us, in case you agree I can open a different PR)

composer.json Show resolved Hide resolved
@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Nov 25, 2019

Thanks for this! Such a shame we didn't manage to meet on SymfonyCon. I could've shown you the idea for new page-object library :(

@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Nov 25, 2019

Also, would you mind to send this PR against the 2.3 branch?

@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 25, 2019

Thanks for this! Such a shame we didn't manage to meet on SymfonyCon. I could've shown you the idea for new page-object library :(

Yeah, what a pity!
Talk about this project in front of a beer would have been great. Maybe we'll get the chance at SymfonyCon2020 or if you apply for PHPDay Italy (I think the CFP is open).
BTW if you want to share some thoughs we can managed this online with slack or whatever.

Gonna change the base of PR ;)

@DonCallisto DonCallisto changed the base branch from master to 2.3 Nov 25, 2019
@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from 54a697e to 286f804 Nov 25, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 25, 2019

Done! :)

@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Nov 26, 2019

Seems that there's no phpspec version that supports PHP 7.4 yet :(

@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 26, 2019

@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 29, 2019

https://github.com/phpspec/phpspec/releases/tag/6.1.0

I think I can also bump PHPSpec version now on go on with this.
WDYT?

@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Nov 29, 2019

Go for it. You'll probably need to drop PHP 7.1, which is fine.

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch 4 times, most recently from c07a2a3 to d2bf64d Nov 30, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 30, 2019

We're still in a showstopper situation, take a look --> BossaConsulting/phpspec-expect#59

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from d2bf64d to c11f2ae Nov 30, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 30, 2019

Nevermind, they merged the PR. Let's check if CI is green.

Edit: Still no new tagged version yet :\

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch 2 times, most recently from c527578 to 35ea221 Nov 30, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 30, 2019

WoW, huge of trouble with this version bumping 😄
Take a look at this FriendsOfPHP/Goutte#393

I know is related to --prefer-lowest composer flag but...

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch 2 times, most recently from a259c04 to 6ce3a67 Nov 30, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Nov 30, 2019

Ok I've done also some other fixes (48f1fbc), now if on Goutte they raise Guzzle version we should be fine otherwise what would you do? I won't like the idea to drop --prefer-lowest check actually.

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from 6ce3a67 to 48f1fbc Nov 30, 2019
.travis.yml Outdated Show resolved Hide resolved
@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from 48f1fbc to 1e19035 Dec 12, 2019
@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch 2 times, most recently from 1945541 to 2a16085 Dec 12, 2019
@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Dec 12, 2019

What if we added a conflict with older versions of guzzle?

@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Dec 12, 2019

How? Don’t understand the question.

@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Dec 12, 2019

I'm talking about adding guzzle to the conflict section of composer.json. Not sure if it impacts --prefer-lowest though.

@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Dec 12, 2019

Umh, don't know if that's gonna work but how about require Guzzle directly in our lib with the right version?
I know it's not a direct dependency but we can mitigate the problem unless a more restrictive version of Goutte will be released.
I really don't like this solution but I think would work.

@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Dec 12, 2019

Yeah, one of the two should work :)

@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from 2a16085 to 614abb6 Dec 12, 2019
@DonCallisto

This comment has been minimized.

Copy link
Contributor Author

DonCallisto commented Dec 12, 2019

conflict works like a charm. Didn't knew about it, thanks for sharing this.
BTW please check if I've introduced too strict version (I think I've not but let's double check)

With php7.4 release, I think is good to try the extension against new version.
@DonCallisto DonCallisto force-pushed the DonCallisto:patch-4 branch from 614abb6 to f4613bf Dec 12, 2019
@jakzal jakzal merged commit b41bf1b into sensiolabs:2.3 Dec 12, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jakzal

This comment has been minimized.

Copy link
Member

jakzal commented Dec 12, 2019

Thank you so much for working on this! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.