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

Upgrade to PHP8.1 #76

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Upgrade to PHP8.1 #76

wants to merge 8 commits into from

Conversation

juampi92
Copy link

@juampi92 juampi92 commented Sep 24, 2022

This PR upgrades the entire package and its dependencies to rely on php 8.1.
The main drive for the php update is to be able to use all latest versions of the existing packages.

Special mention

This specific commit prevents PHP Enum classes from breaking.

It used to break saying that $node->isAbstract() doesn't exist for Node\Stmt\Enum_.

This PR includes

  • Added github actions to include tests
  • Updated PHPUnit assertions to work with phpunit 9.5
  • Updated php-cs-fixer --rules label. And updated the codebase afterwards with the new psr12 standard.
  • $method->getReturnType()->getName() update.
  • The Dockerfile is also updated to use PHP8.1 too.

There are some failing tests that I can't make it work with github actions:

  • Install json.so extension: I added json to the list of extensions, but it didn't seem to work.
  • I added a new test for Enums, and now there is a new error for the test. I couldn't figure out how the tests well too well, but I have a suspicion that the Enums are not printing anything. I run this updated version against my codebase and it was able to generate the Text version correctly.

I opened it as a draft to get feedback and assistance before committing to fix the tests. Thanks in advance for this package and for the help!

@mihaeu
Copy link
Owner

mihaeu commented Oct 8, 2022

Hey, thank you so much for this PR and sorry for letting you wait this long. Work's been very busy, but I'm getting on it right now :)

COPY . /dephpend
WORKDIR /dephpend

RUN curl https://raw.githubusercontent.com/composer/getcomposer.org/76a7060ccb93902cd7576b67264ad91c8a2700e2/web/installer | php -- --quiet \
&& php -n composer.phar install
Copy link

Choose a reason for hiding this comment

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

If we are at the place, what do you think, if we replace the installer part with e.G.:

COPY --from=composer:2.4.4 /usr/bin/composer /usr/bin/composer

@djschilling
Copy link

@mihaeu what is the progress here?

@pierrekttipay
Copy link

@mihaeu please? 1.5 years for a PR that important is way too much ;-)

@tellojsu
Copy link

@mihaeu Necesitas ayuda? Need help? would love to see this get pushed through

@vasanth-kumar-m-y
Copy link

@juampi92 @mihaeu any update on this PR or PHP 8.1 support

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.

None yet

7 participants