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.

Sign up for GitHub

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

Jump to bottom

CI: added PHP 8.4 #723

Merged
merged 7 commits into from
Oct 24, 2024
Merged

CI: added PHP 8.4 #723

merged 7 commits into from
Oct 24, 2024

Conversation

k00ni
Copy link
Collaborator

@k00ni k00ni commented Jun 26, 2024 •
edited
Loading

Type of pull request

  • New feature (involves code and configuration changes)

About

PHP 8.4 is still in alpha so there is no need to hurry. I wanted to have it integrated already so we can observe changes and how PDFParser behaves.

@k00ni k00ni self-assigned this Jun 26, 2024
@k00ni k00ni changed the title CI added PHP 8.4 to the list CI: added PHP 8.4 Jun 26, 2024
@k00ni k00ni marked this pull request as ready for review September 18, 2024 14:27
@k00ni k00ni requested review from j0k3r and smalot September 18, 2024 14:28
@k00ni
Copy link
Collaborator Author

k00ni commented Sep 18, 2024 •
edited
Loading

@j0k3r What do you think? I saw a few warnings in PHP 8.4 test report and will have a look later on.

j0k3r
j0k3r approved these changes Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Good idea 👍🏼

@k00ni
Copy link
Collaborator Author

k00ni commented Sep 25, 2024 •
edited
Loading

Github has hiccups currently... Saw and still see these errors when running tests. First it seems they all failed but in the end almost all went through.

/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules 
          --depth=1 origin +bb7464be22837761d081eb3502fd349ff44c7dde:refs/remotes/pull/723/merge
  remote: git: 'lariat' is not a git command. See 'git --help'.        
  remote: aborting due to possible repository corruption on the remote side.

Bildschirmfoto vom 2024-09-25 16-48-49

Stopped working on the PR for now, because I want to see the hidden warnings when running tests using PHP 8.4 and can't reach test logs because of these weird errors.

@k00ni
Merge branch 'master' into feature/php-8.4-support
21f7d8b
@k00ni
fixed warnings in Font.php
8cebc93 
Warnings were:

1) /var/www/html/src/Smalot/PdfParser/Font.php:292
range(): Argument #1 ($start) must not be empty, casted to 0

Triggered by:

* PHPUnitTests\Integration\FontTest::testCalculateTextWidthNoWidthsKey
  /var/www/html/tests/PHPUnit/Integration/FontTest.php:473

2) /var/www/html/src/Smalot/PdfParser/Font.php:292
range(): Argument #2 ($end) must not be empty, casted to 0

Triggered by:

* PHPUnitTests\Integration\FontTest::testCalculateTextWidthNoWidthsKey
  /var/www/html/tests/PHPUnit/Integration/FontTest.php:473
@k00ni
fixed 3 CS issues
fabd628
@k00ni
Merge branch 'master' into feature/php-8.4-support
304cd3d
@k00ni
PDFObject: fixed introduced syntax errors in older PHP versions; refi…
eb4e47a 
…ned comment in Font.php
@k00ni
Copy link
Collaborator Author

k00ni commented Oct 24, 2024 •
edited
Loading

Summary:

  • Nothing spectacular anymore
  • One adaption required to avoid a warning for array_flip (FYI)
  • PHPUnit complains about some invalid attributes (I don't really care about that, because its neither PDFParser nor quality related and they changed the config again and again from version to version. We have to use 3 versions for now to cover PHP 7.2 until 8.4, so its normal that we stumble upon deprecations)

In the following the output of PHPUnit (10.x) when running on PHP 8.4 RC2:

Runtime:       PHP 8.4.0RC2
Configuration: /var/www/html/phpunit.xml

...............................................................  63 / 187 ( 33%)
............................................................... 126 / 187 ( 67%)
.............................................................   187 / 187 (100%)

Time: 00:14.655, Memory: 605.75 MB

There was 1 PHPUnit test runner warning:

1) Test results may not be as expected because the XML configuration file did not pass validation:

  Line 5:
  - Element 'include': This element is not expected.

CC @j0k3r Any objection or can we merge it?

@j0k3r
Copy link
Collaborator

j0k3r commented Oct 24, 2024

It's ok!

@k00ni k00ni merged commit f8ae4b4 into master Oct 24, 2024
62 checks passed
@k00ni k00ni deleted the feature/php-8.4-support branch October 24, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j0k3r j0k3r j0k3r approved these changes

@smalot smalot Awaiting requested review from smalot

Assignees

@k00ni k00ni

Labels
enhancement unit tests / CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k00ni @j0k3r