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.x] Add code checks #468

Open
alexislefebvre opened this issue Nov 16, 2018 · 5 comments
Open

[2.x] Add code checks #468

alexislefebvre opened this issue Nov 16, 2018 · 5 comments
Assignees
Milestone

Comments

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented Nov 16, 2018

Add jobs in Travis CI for:

  • PHP-CS-Fixer
  • PHPStan
  • PHPMD ?

Code analysis is fast but having two different jobs will make results easier to read if there's an error.

We could rely on Docker image https://github.com/jakzal/phpqa instead of adding dependencies to dev.

Some ideas for the configuration of these tools:

Assigned to @Jean85

@alexislefebvre alexislefebvre changed the title Add code checks [2.0] Add code checks Nov 16, 2018
@alexislefebvre alexislefebvre changed the title [2.0] Add code checks [2.x] Add code checks Nov 16, 2018
@Jean85
Copy link
Contributor

Jean85 commented Nov 16, 2018

Why you don't want to rely on require-dev? IMO it's hard for contributors later, since they have to run additional commands to run the checks locally...

@alexislefebvre
Copy link
Collaborator Author

Using Docker was a suggestion. You're right, it's easier to install analysis tools with Composer.

@Jean85
Copy link
Contributor

Jean85 commented Nov 16, 2018

I was trying to work on this but it seems a big mess: we have too many dev dependencies which I cannot skip or the analysis will fail, and at the same time PHPCR doesn't allow PHPUnit 7, which is required to use phpstan/phpstan-phpunit...

It seems to me that we're having too many optional dependencies in this package. Maybe we should split them?

@alexislefebvre
Copy link
Collaborator Author

we have too many dev dependencies which I cannot skip or the analysis will fail

Could you please explain this? What is failing?

About PHPCR, we may drop it from this bundle, there have been no issue about PHPCR during five years, maybe it's not used much.

@Jean85
Copy link
Contributor

Jean85 commented Nov 17, 2018

PHPStan requires all used dependencies to be installed because it need to know all symbols used in our code.

@alexislefebvre alexislefebvre added this to the 2.0 milestone Nov 24, 2018
@alexislefebvre alexislefebvre self-assigned this Jun 2, 2019
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

No branches or pull requests

2 participants