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

[2.0] Fix runCommand PHPDoc #467

Merged
merged 1 commit into from
Nov 15, 2018
Merged

[2.0] Fix runCommand PHPDoc #467

merged 1 commit into from
Nov 15, 2018

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Nov 15, 2018

This BC was introduced but hidden to static analyzers due to this PHPDoc error.

Maybe we should introduce static analysis in 2.0 CI?

@alexislefebvre
Copy link
Collaborator

Yes, adding static analysis tools is a great idea. FlintCI was added in #371 but I don't know why it doesn't work anymore (this PR has no check from FlintCI).

About this line, the whole PHPDoc block is redundant for this method, maybe we should drop it when it adds no value to the args and return type hints.

@alexislefebvre alexislefebvre merged commit ee1b91e into liip:2.x Nov 15, 2018
@Jean85 Jean85 deleted the patch-1 branch November 16, 2018 09:05
@Jean85
Copy link
Contributor Author

Jean85 commented Nov 16, 2018

  • FlintCI is not working and neither Travis, we're out of CI checks! 😞 It seems to have stopped working evend mid-PR in Dependencies upgrade #379
  • superfluous PHPDoc could be suppressed using PHP-CS-Fixer, with the `` rule
  • on that regard, I don't see PHP-CS-Fixer required in dev or even run in CI; I see that we have StyleCI, but the config is out of sync with the committed one, that's not good!
  • I can introduce PHPStan in the CI checks, I'll work on a PR ASAP; I think we should move the CS check there too, leveraging Travis' stages

@alexislefebvre
Copy link
Collaborator

Great news Jean, see #468.

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