Skip to content

Update dependencies and update to reflect new PHP Parser changes.#16

Merged
oscarotero merged 4 commits intophp-gettext:masterfrom
BusterNeece:master
Mar 10, 2024
Merged

Update dependencies and update to reflect new PHP Parser changes.#16
oscarotero merged 4 commits intophp-gettext:masterfrom
BusterNeece:master

Conversation

@BusterNeece
Copy link
Copy Markdown
Contributor

This PR changes the following:

  • Updates composer.json to depend on nikic/php-parser version 5.x instead of 4.x
  • Bump the minimum PHP version from 7.2 to 7.4 (php-parser itself only supports 7.4 and up)
  • Bump the versions of the testing dependencies
  • Update factory instantiation to reflect php-parser's new syntax
  • Slightly update how comments are parsed to handle changes in the parser

Copy link
Copy Markdown
Member

@oscarotero oscarotero left a comment

Choose a reason for hiding this comment

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

Thanks!
I just left a couple of comments to fix a bug regarding numeric arguments.

Comment thread tests/PhpFunctionsScannerTest.php Outdated
Comment thread tests/PhpFunctionsScannerTest.php Outdated
@BusterNeece
Copy link
Copy Markdown
Contributor Author

@oscarotero Issues with numeric args should be fixed now. I initially couldn't tell what was causing that, but it turns out it was a simple problem with a simple fix.

@BusterNeece
Copy link
Copy Markdown
Contributor Author

@oscarotero Just pushed an update that should fix the GitHub Actions, setting them to run from 7.4 up, and downgrading PHPUnit to the latest stable version that supports that far back. You could certainly set the minimum PHP requirement higher, but I wanted to have as few BC breaks as possible in the PR.

@oscarotero oscarotero merged commit 2e342de into php-gettext:master Mar 10, 2024
@oscarotero
Copy link
Copy Markdown
Member

Great work!
Thank you!

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