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

Lock minimum dependencies and work around circular dependency for tests #71

Merged
merged 2 commits into from
Aug 9, 2017

Conversation

clue
Copy link
Member

@clue clue commented Aug 8, 2017

The react/dns package currently has a dependency on react/socket and thus has no way of knowing which version of said package is compatible with this root package version. Here's a dependency graph that shows this cyclic dependency (generated via https://github.com/clue/graph-composer):

orig

The dependency graph highlights how Composer picks an older version of react/socket by default which does not have this dependency. Once you actively pass the root package version, Composer will actually pick the "right" version:

proper

Note that this only affects this package when it's installed as the root package (for testing and development). Everything works without this when it's used as a normal dependency.

Refs reactphp/socket#54
Builds on top of #70.

We currently have a cyclic dependency between react/socket and react/dns
which causes the test setup to fail or pick outdated versions otherwise.
@clue clue added this to the v0.4.10 milestone Aug 8, 2017
@WyriHaximus
Copy link
Member

WyriHaximus commented Aug 8, 2017

The graph seems to be missing...

@clue clue changed the title Lock minimum dependencies and add work around circular dependency for tests Lock minimum dependencies and work around circular dependency for tests Aug 8, 2017
@clue
Copy link
Member Author

clue commented Aug 8, 2017

Sorry, accidentally hit submit while typing, post updated :-)

@WyriHaximus
Copy link
Member

It happens no problem 👍

@WyriHaximus WyriHaximus merged commit c713445 into reactphp:master Aug 9, 2017
@clue clue deleted the deps branch August 9, 2017 07:11
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.

3 participants