-
Notifications
You must be signed in to change notification settings - Fork 1
deps: update symfony dependencies to ^7 #2
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
base: main
Are you sure you want to change the base?
Conversation
igornast
commented
Jul 2, 2025
- update symfony http-client to version ^7
- add php ^8.2 requirement
- update readme
* update symfony http-client to version ^7 * add php ^8.2 requirement * update readme
@Einenlum could you please take a look? 👀 |
Sorry, I missed this! I'm on holiday but I can definitely take a look at it on August the 5th |
"nyholm/psr7": "^1.8" | ||
}, | ||
"scripts": { | ||
"unit-test": "phpunit tests", | ||
"static-analysis": "phpstan", | ||
"test-cs": "./vendor/bin/php-cs-fixer fix --dry-run --diff -v", | ||
"test-cs-fix": "./vendor/bin/php-cs-fixer fix --diff -v", |
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.
Maybe just cs-fix
or fix-cs
here?
@@ -42,11 +42,11 @@ public function it_says_if_a_directory_does_not_exist(): void | |||
} | |||
|
|||
/** @test */ | |||
public function it_list_files_in_a_directory(): void | |||
public function itListFilesInADirectory(): void |
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.
Could we maybe add this configuration rule to php cs fixer?
https://cs.symfony.com/doc/rules/php_unit/php_unit_method_casing.html
with the snake_case
value?
I find it way more readable for long sentences like this in tests. But maybe it's just me, it's not critical.
use Einenlum\PhpStackDetector\Stack; | ||
use Einenlum\PhpStackDetector\StackType; | ||
|
||
abstract class BaseComposerTypeDetector |
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.
I tend to use traits and interfaces rather than relying on abstract classes which provides better composition, but if it's required I'm okay with it too :)
); | ||
|
||
if (null === $version) { | ||
return null; |
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.
So no Stack returned if the version is null?
case TYPO3_CMS = 'typo3-cms'; | ||
case CAKE_PHP = 'cakephp'; | ||
case GRAV_CMS = 'grav-cms'; | ||
case CODEIGNITER = 'codeigniter'; |
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.
I realize it would be probably better to order all these stacks alphabetically
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.
@igornast
A few comments/questions, but looks good overall
Nice work ❤️ !