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

#6840 Report useless array filter #1076

Closed
wants to merge 17 commits into from

Conversation

leongersen
Copy link
Contributor

@leongersen leongersen commented Mar 16, 2022

(I don't have access to PHP8 right now, I'll have to fix the build in the CLI)

Implements phpstan/phpstan#6840

@leongersen leongersen force-pushed the useless-array-filter branch 2 times, most recently from ac3e420 to 25797c9 Compare March 16, 2022 10:45
return [];
}

$args = array_values($node->getArgs());
Copy link
Contributor

Choose a reason for hiding this comment

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

is the array_values call necessary?

@staabm
Copy link
Contributor

staabm commented Mar 16, 2022

I guess this PR should target the 1.4.x branch

@leongersen leongersen changed the base branch from 1.5.x to 1.4.x March 16, 2022 10:58
@leongersen
Copy link
Contributor Author

I've created a new PR to 1.4.x: #1077

@leongersen leongersen closed this Mar 16, 2022
@leongersen leongersen deleted the useless-array-filter branch March 16, 2022 11:25
@herndlm
Copy link
Contributor

herndlm commented Mar 16, 2022

Not entirely sure, if it really should target the 1.4.x branch, but regarding the rebase: I think an interactive rebase would have worked where you have to manually drop all the new commits from 1.5.x. In case you need to do something like that again :)

@ondrejmirtes
Copy link
Member

Hi, it's unfortunate but you should always evaluate every review with critical thinking :) This is absolutely a new feature, hence it should target 1.5.x

@leongersen leongersen restored the useless-array-filter branch March 16, 2022 11:41
@leongersen
Copy link
Contributor Author

No problem! I'm learning things along the way 😄 . I can't reopen this PR, but I've rebased the new one to 1.5.x.

image

@staabm
Copy link
Contributor

staabm commented Mar 16, 2022

Hi, it's unfortunate but you should always evaluate every review with critical thinking :) This is absolutely a new feature, hence it should target 1.5.x

Ok it seems we have a different understanding about the branches and what is considered a feature or not.

IMO #1067 was also a new feature 🤷‍♂️

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