zend-mvc icon indicating copy to clipboard operation
zend-mvc copied to clipboard

Published 20 hours ago • verified publisher icon zendframework

PHPStan fixes

Open thomasvargiu opened this issue 6 years ago • 7 comments

In order to bump to PHP 7 minimum version in the future, I used phpstan to static analyse the code, correcting some phpdoc errors and adding guards throwing exceptions when needed. Code quality will be improved when other components will be improved.

It's just a first step, but I think it will be useful.

I don't know if this could be considered a BC break, probably it isn't.

thomasvargiu avatar Oct 13 '18 13:10 thomasvargiu

@Ocramius I can add phpstan on CI, but just a low level. Too many errors depends on other libraries.

thomasvargiu avatar Oct 15 '18 13:10 thomasvargiu

@thomasvargiu that's fine - we can raise the level as we get better

Ocramius avatar Oct 15 '18 13:10 Ocramius

@thomasvargiu btw, I still suggest getting rid of all the throw new SomeException($someString), and having throw SomeException::unexpectedType($expected, $actual) instead

Ocramius avatar Oct 15 '18 14:10 Ocramius

@Ocramius I'm still working on this. I set this PR as WIP

thomasvargiu avatar Oct 15 '18 14:10 thomasvargiu

Done. Needs a complete review

thomasvargiu avatar Oct 15 '18 18:10 thomasvargiu

This repository has been closed and moved to laminas/laminas-mvc; a new issue has been opened at https://github.com/laminas/laminas-mvc/issues/9.

weierophinney avatar Dec 31 '19 21:12 weierophinney

This repository has been moved to laminas/laminas-mvc. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone laminas/laminas-mvc to another directory.
  • Copy the files from the second bullet point to the clone of laminas/laminas-mvc.
  • In your clone of laminas/laminas-mvc, commit the files, push to your fork, and open the new PR. We will be providing tooling via laminas/laminas-migration soon to help automate the process.

weierophinney avatar Dec 31 '19 21:12 weierophinney