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

composer: drop dependency on doctrine/* packages [closes #7] #9

Closed
wants to merge 2 commits into from

Conversation

JanTvrdik
Copy link
Contributor

No description provided.

@ondrejmirtes
Copy link
Member

No, sorry, I feel like there need to be constraints in case something work differently in new (breaking) versions of Doctrine.

@JanTvrdik
Copy link
Contributor Author

I understand the problem it is trying to solve but it is extremely bad solution.

  1. It is a lie. The library claims to require files it does not actually require and by design MUST NOT require (depending on the files would mean it's not doing static analysis). It would be better to use conflict (with inverse version range) instead of require.

  2. It breaks the entire analysis when phpstan is installed correctly the way tools are supposed to be installed (i.e. not in project vendor directory, for example with usage of composer-bin-plugin plugin), because it installs different version of Doctrine than you application is using and uses the wrong version of doctrine for analysis.

This is what phpstan installation looks on darujme.cz

image

Without manually writing the replace section and thus manually killing broken require from phpstan/phpstan-doctrine it would not work at all.

@ondrejmirtes ondrejmirtes reopened this Oct 27, 2017
@ondrejmirtes
Copy link
Member

OK, I will take some time and try to reconsider this.

@hkdobrev
Copy link
Contributor

👍 on this. @JanTvrdik Perhaps you could resolve the conflicts and add the conflict section you've mentioned? Thank you!

@ondrejmirtes
Copy link
Member

Superseded by #29.

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.

3 participants