Skip to content

fix for #2 & #3 (also updates SniffTest.php) #4

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

Closed
wants to merge 1 commit into from

Conversation

bkdotcom
Copy link

@bkdotcom bkdotcom commented Oct 26, 2021

refactor MaximumComplexitySniff and Analyzer to utilize "$phpcsFile" and how nestingIncrement is calculated

@Rarst
Copy link
Owner

Rarst commented Oct 26, 2021

Hey, thanks for PR! This looks to touch a lot in tests (I saw you mentioned you had trouble), with some cosmetic changes on top, and tests failed in Scrutinizer after.

Since it's all one commit it would be hard for me to separate out what happens why.

Would you mind making PR with just your code changes and adding a new unit test for them, without changing how unit tests work? It's ok if tests don't work at that point, I will look into what's wrong with them, but that's tangential to code changes.

If not I will try sort it out of this, but will probably take me longer to get to that. :)

@bkdotcom
Copy link
Author

bkdotcom commented Oct 26, 2021

@Rarst : I've created #5 without the update to SniffTest

However It's still necessary to modify AnalyzerTest
reason
Analyzer::computeForFunctionFromTokensAndPosition's signature changed to
to public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $position): int
(previously first param was an array of tokens)

@bkdotcom bkdotcom changed the title refactor MaximumComplexitySniff and Analyzer to utilize "$phpcsFile" … fix for #2 & #3 (also updates SniffTest.php) Oct 26, 2021
@Rarst
Copy link
Owner

Rarst commented Nov 1, 2021

Replaced by #5

@Rarst Rarst closed this Nov 1, 2021
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