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

Allow psr/container v2 #1206

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Allow psr/container v2 #1206

merged 1 commit into from
Nov 11, 2021

Conversation

ADmad
Copy link
Contributor

@ADmad ADmad commented Oct 20, 2021

No description provided.

@othercorey
Copy link

Is this something enqueue can support? We're hoping it doesn't require a major version bump since it's compatible.

@Steveb-p
Copy link
Contributor

Is this something enqueue can support? We're hoping it doesn't require a major version bump since it's compatible.

Looking at the codebase changes in the package in question (php-fig/container@1.1.1...master) there should be no need for a major version bump.

@ADmad
Copy link
Contributor Author

ADmad commented Oct 23, 2021

Indeed there isn't. Just need someone to merge this and do a new release :)

@CriztianiX
Copy link

News for this?

@othercorey
Copy link

@makasim Could you review this please?

@makasim
Copy link
Member

makasim commented Nov 2, 2021

could you sync with the latest master branch ? there are some fixes for the tests. Would be good to merge it once it green

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ADmad
Copy link
Contributor Author

ADmad commented Nov 2, 2021

could you sync with the latest master branch ?

Done

@ADmad
Copy link
Contributor Author

ADmad commented Nov 8, 2021

Ping

@makasim
Copy link
Member

makasim commented Nov 8, 2021

There is a failed CI job

@ADmad
Copy link
Contributor Author

ADmad commented Nov 8, 2021

Well I have no idea how this change could be affecting that test. Can you just try re-running the testsuite? Could be a flaky test or an issue with the CI environment.

@makasim
Copy link
Member

makasim commented Nov 8, 2021

I've tried, no luck.

@othercorey
Copy link

This is the failure. Does it look familiar to you?

1) Enqueue\Dbal\Tests\Spec\Mysql\DbalSubscriptionConsumerConsumeFromAllSubscribedQueuesTest::test
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => 'fooBody'
-    1 => 'barBody'
 )

@othercorey
Copy link

@makasim We cannot find any connection to the changes in psr/container v2 for this test. I'm not sure what you expect us to do to resolve this. Can we run the tests again?

@makasim
Copy link
Member

makasim commented Nov 11, 2021

I re-run the tests. it might very well be not related to your changes. though CI must be green before I can merge otherwise it would soon go out of control.

@othercorey
Copy link

I re-run the tests. it might very well be not related to your changes. though CI must be green before I can merge otherwise it would soon go out of control.

We appreciate that need. I had a hard time following that specific test so I wasn't sure how to help.

@makasim
Copy link
Member

makasim commented Nov 11, 2021

That's indeed a wired one. Recently there was a PR #1211 that addressed issues with the tests. And it was green.

@othercorey
Copy link

This appears to pass now. Hope that's enough.

@makasim makasim merged commit bb4a036 into php-enqueue:master Nov 11, 2021
@ADmad ADmad deleted the container-v2 branch November 11, 2021 08:13
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.

None yet

5 participants