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

feat: php 8 support #43

Merged
merged 6 commits into from
Mar 31, 2021
Merged

feat: php 8 support #43

merged 6 commits into from
Mar 31, 2021

Conversation

ndench
Copy link
Contributor

@ndench ndench commented Mar 14, 2021

Just copied the nexylan/slack#70 pull request.

@ndench
Copy link
Contributor Author

ndench commented Mar 14, 2021

Symfony changed it configuration exception messages when updating to 5.0, so I had to modify a test. I dropped PHP 7.3 support because nexylan/slack#70 did the same.

The --prefer-lowest build is failing because it's installing PHPUnit 8.0 which seems to not be working with xdebug 3. I'm not sure how to fix this.

@soullivaneuh
Copy link
Contributor

@ndench Thanks! Can you allow me edition on this PR so I can take a look and finish it?

@soullivaneuh soullivaneuh marked this pull request as draft March 15, 2021 09:59
@ndench
Copy link
Contributor Author

ndench commented Mar 15, 2021

No problem @soullivaneuh. I've invited you to my fork as a maintainer, feel free to make any changes necessary. I'm happy to help with anything you need as well.

@soullivaneuh
Copy link
Contributor

soullivaneuh commented Mar 15, 2021

@ndench
Copy link
Contributor Author

ndench commented Mar 15, 2021

Unfortunately allowing edits from maintainers appears to only work if the fork is created from a user account. However I created this fork from an organisation account so that checkbox is not available for me. See https://github.community/t/how-can-we-enable-allow-edits-from-maintainers-by-default/2847/6

I do not see the checkbox shown in the docs link:
image

Feel free to edit as a maintainer, or even create a separate branch and pull in my commits if you like.
I could also re-fork the repository if that's easier too?

@ndench
Copy link
Contributor Author

ndench commented Mar 28, 2021

@soullivaneuh I got the test passing. The --prefer-lowest option was installing a version of php-code-coverage which wasn't reading the xdebug mode properly. See this issue. It was also installing PHPUnit 8.0 which doesn't have the new expectExceptionMessageMatches function which was introduced it 8.4.

We can solve these by:

  • Specifying XDEBUG_MODE=coverage as mentioned in the linked issue above
  • Requiring PHPUnit minimum 8.4

@soullivaneuh
Copy link
Contributor

Hello,

Thanks for giving some time on it! 👍

Looks goot to me.

@soullivaneuh soullivaneuh marked this pull request as ready for review March 31, 2021 13:18
@soullivaneuh soullivaneuh reopened this Mar 31, 2021
@soullivaneuh soullivaneuh merged commit e56d131 into nexylan:master Mar 31, 2021
@ndench ndench deleted the php-8 branch April 1, 2021 22: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.

2 participants