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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce static analysis #9035

Closed
wants to merge 2 commits into from
Closed

Introduce static analysis #9035

wants to merge 2 commits into from

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Jun 1, 2019

Hello silverstripers! :)

This is the first round of adding another 100 pair of 馃憖 (phpstan) to your project.
I do like your modern technology but there are dirt spots.

What do you think?

@@ -86,10 +82,6 @@ class DebugView
'title' => 'User Deprecated',
'class' => 'notice'
),
E_CORE_ERROR => array(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These 2 are doubles.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jun 1, 2019

BTW These are some of the Level 0 errors. Please consider going up to Level 4 (or 7 馃殌)

@robbieaverill
Copy link
Contributor

Hey @szepeviktor, thank you for the pull request! All the changes look good to me, the only thing I'm hesitant about is introducing phpstan. We discussed this last year and at the time decided not to. Moving forwards, it might be worth reopening that RFC again since I think phpstan offers a lot, but for now perhaps you could remove it from composer.json and keep the rest of your changes? Thanks very much!

@szepeviktor szepeviktor changed the base branch from 4 to master June 3, 2019 22:15
@szepeviktor szepeviktor changed the base branch from master to 4 June 3, 2019 22:15
@szepeviktor
Copy link
Contributor Author

Case closed :)

@szepeviktor szepeviktor closed this Jun 3, 2019
@szepeviktor szepeviktor deleted the phpstan-1st-round branch June 3, 2019 23:04
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.

None yet

3 participants