Skip to content
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

Classes/DeclarationCompatibility: remove redundant code #768

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Aug 23, 2023

The $functionList property is only ever written to, never read, so all that work walking all the tokens in the complete class scope is redundant (and was done in a highly inefficient way in the first place).

So, removing all code related to the property should improve performance of the sniff a little.

The `$functionList` property is only ever written to, never _read_, so all that work walking all the tokens in the complete class scope is redundant (and was done in a highly inefficient way in the first place).

So, removing all code related to the property should improve performance of the sniff a little.
@jrfnl jrfnl added this to the 2.3.4 milestone Aug 23, 2023
@jrfnl jrfnl requested a review from a team as a code owner August 23, 2023 19:31
Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this was meant to be used in a later development.

@GaryJones GaryJones merged commit 02abdba into develop Aug 23, 2023
32 checks passed
@GaryJones GaryJones deleted the feature/declarationcompatibility-performance-fix branch August 23, 2023 19:58
@jrfnl
Copy link
Collaborator Author

jrfnl commented Aug 23, 2023

I wonder if this was meant to be used in a later development.

Quite possibly or a left-over of something removed.

I did try to do a trace back, but it came in with the initial import from SVN, so I couldn't trace the history back any further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants