Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PHPCS has been really slow, it takes about twice as long as usual. See for example this run of 1m 23s compared to this run of 2. 58s.
Turns out the rule
Spryker.Internal.SprykerDisallowFunctionsrequires a PHP version or has to be turned off, otherwise it goes nuts. In the file, it just says:And looking at the code, "core" means Spryker core:
and
There are lots of string operations involved in
getNamespace(), and I think those are run for every single token, causing the issue.Similar problem with
Spryker.Internal.SprykerPreferStaticOverSelf, now I kicked out all of them (except SprykerDisallowFunctions, which could be useful and is cheap when configured - 0.004s in Criteria).The change to PHP version in SprykerDisallowFunctions should cause an error when functions from later PHP versions are used. So setting it to 8.1 would be wrong, since we are using symfony polyfills. However, there is no error when using
array_first(), which was added in PHP 8.5. Don't know if the rule recognizes the polyfill? Does it even do anything? Maybe I should have just disabled it...