Skip to content

Conversation

@viktorprogger
Copy link
Contributor

@viktorprogger viktorprogger commented Jan 27, 2020

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Tests pass?
Fixed issues

This PR potentially replaces another one.

@viktorprogger viktorprogger requested a review from a team January 27, 2020 14:40
@viktorprogger viktorprogger added the status:code review The pull request needs review. label Jan 27, 2020
@viktorprogger viktorprogger mentioned this pull request Jan 27, 2020
@yiiliveext
Copy link
Contributor

Tests: 26, Assertions: 61, Errors: 2, Failures: 5.

@viktorprogger
Copy link
Contributor Author

viktorprogger commented Jan 27, 2020

@yiiliveext, yes, I know it. It is not related to this PR. See issue #6.

@yiiliveext
Copy link
Contributor

@yiiliveext, yes, I know it. It is not related to this PR. See issue #6.

See my alternative PR (#8).
It passes your test.

@viktorprogger
Copy link
Contributor Author

viktorprogger commented Jan 27, 2020

Maybe #8 is even better than this PR because it allows to check not only explicitly set definitions.

So, as conclusion I can say that there is really no need to use CompositeContainer.

@yiiliveext
Copy link
Contributor

Maybe #8 is event better than this PR because it allows to check not only explicitly set definitions.

I agree. On my PR all tests have been passing.

So, as conclusion I can say that there is really no need to use CompositeContainer.

I agree you are right.

@viktorprogger viktorprogger deleted the fix-resolving-dependencies branch February 3, 2020 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants