Skip to content

Conversation

@EmilMassey
Copy link
Contributor

Currently CI checks fail due to code style issues being reported by the latest version of CS fixer. This PR fixes these issues and also changes configuration for trailing_comma_in_multiline fixer not to add comma after parameters, because it's not allowed in PHP 7.

@AlessandroMinoccheri
Copy link
Member

Hi @EmilMassey is it possible to fix pipelines?

@EmilMassey
Copy link
Contributor Author

EmilMassey commented Jan 11, 2025

Hi @AlessandroMinoccheri Does it make sense to run coding standard checks for PHP < 8.0? I added condition to skip this step in the pipelines, because the problem is that depending on PHP version, another CS Fixer version is installed and the configurations for both of them differ. Alternatively we could define different configuration options depending on the PHP version.

Also there's unrelated job failed for PHP 7.1 due to roave/security-advisories disallowing installation of vulnerable version of symfony/process: https://symfony.com/blog/cve-2024-51736-command-execution-hijack-on-windows-with-process-class. It seems update is not available for PHP 7.1, so maybe support for PHP 7.1 should be dropped altogether?

@micheleorselli
Copy link
Member

@EmilMassey we pinned phpcsfixer to a specific version so we should not get randomic results based on phpcsfixer and php version combinations. That said, I would like to bump phpcsfixer to the latest version and update the codebase to add return types. I would say this PR can be closed, what do you think?

@EmilMassey
Copy link
Contributor Author

Yes, I created this PR only to fix the failing CI checks, but bumping the CS Fixer and updating the codebase is definitely the way to go here. Thank you.

@EmilMassey EmilMassey closed this Feb 18, 2025
@EmilMassey EmilMassey deleted the code-style-fixes branch February 18, 2025 06:48
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.

3 participants