-
Notifications
You must be signed in to change notification settings - Fork 4
add T_MATCH to increments #8
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
Conversation
jrfnl
commented
Mar 22, 2023
lift dependency of PHP version from ^7.2 to ^8.0 (where T_MATCH is introduced)
That sound completely unnecessary to me. PHP_CodeSniffer backfills the T_MATCH token since PHPCS 3.6.0 (all the way back to PHP 5.4). So the only thing needed would be to raise the minimum PHPCS version to 3.6.0 (or later).
Applied suggested changes to phpunit.xml using vendor/bin/phpunit tests --migrate-configuration
Not a good idea as that will prevent the tests from running on PHPUnit < 9.3 and with a PHP minimum of PHP 7.2, PHPUnit 8.x is still needed.
I was not aware of the backwards compatibility in PHPCS. I removed the unnecessary upgrades
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please remove Docker files, I am not sure why would they be necessary or relevant to the PR.
- I've approved the test to run for PR and setup is currently crashing over PHPUnit 9 being required in the changed lock and not compatible with composer.json.
- Actually please drop requiring PHPUnit. It's not relevant to the enhancement and I don't have it in me to ponder changes to the setup right now.
Adding PHPUnit and the dockerfiles were necessary for me to get anything working locally, specifically composer install and phpunit tests.
I did now add the docker files to my excludes so that they are no longer added to the repo. As for PHPunit, I removed that but can no longer test locally. I suppose there is some way to install and use it globally, will investigate this.
Anyway I removed the unnecessary changes
Cheers!
FYI personally I have multiple versions of PHPUnit set up as PHARs in the system. I can have a lot of projects around, and sometimes within same IDE session, so zillion copies of PHPUnit can get hard on IDE and making sense of what's going on. More so if tests are supposed to work with multiple PHPUnit versions.
Uh oh!
There was an error while loading. Please reload this page.
Subject:
I would like to use this package to report cognitive complexity.
In my codebase I use
match()a lot, which was introduced in PHP 8.0.It is similar to switch and from what I can tell, should get the same treatment.
Changes:
(削除) Additional changes (necessary): (削除ここまで)(削除) lift dependency of PHP version from ^7.2 to ^8.0 (where T_MATCH is introduced) (削除ここまで)(削除) Additional changes (optional): (削除ここまで)(削除) Added a Dockerfile and docker-compose.yml for easy local testing (using composer image) (削除ここまで)(削除) Added phpunit to dev-dependencies for local testing (削除ここまで)(削除) Applied suggested changes to phpunit.xml usingvendor/bin/phpunit tests --migrate-configuration(削除ここまで)