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

Make module compatible with Magento 2.4 #10

Merged
merged 1 commit into from
Aug 2, 2020
Merged

Make module compatible with Magento 2.4 #10

merged 1 commit into from
Aug 2, 2020

Conversation

mbijnsdorp
Copy link

@mbijnsdorp mbijnsdorp commented Jul 30, 2020

  • add @var to unit test (phpstan threw errors)
  • update composer dependencies according to Magento 2.4
  • update unit test to make it compatible

Due to the phpunit change, it's no longer backwards compatible with earlier versions of Magento. Let me know If you want to have additional changes or so because of it.

@hostep
Copy link
Member

hostep commented Jul 30, 2020

Thanks @mbijnsdorp!

I was planning on investigating compatibility with Magento 2.4.0 later this week, this PR will help me with that!
I'll try to check it out in the coming days and get back to you!

@hostep hostep merged commit 76b6107 into baldwin-agency:v1-develop Aug 2, 2020
@hostep
Copy link
Member

hostep commented Aug 2, 2020

Thanks for the PR @mbijnsdorp: it mostly looks fine, but I'll add some changes:

  • /** @var MockObject $productCollectionMock */ instead of /** @var \PHPUnit\Framework\MockObject\MockObject $productCollectionMock */
  • I'll update the PHPCompatibility tests, so it ignores the Tests in this module, since they are now no longer compatible with PHP 7.0 due to the added void return type, which is only supported in PHP 7.1 and higher

I'll create some extra commits to fix these things, then do a final review and do some manual tests. When everything looks fine, I'll tag a new release.

Thanks again!

@hostep
Copy link
Member

hostep commented Aug 2, 2020

@mbijnsdorp: unfortunately I found a regression in 2.4.0 with this module which needs additional research: #11

So I won't be tagging a new version until we've figured out a solution for that issue. I hope it won't take long to figure that out! 🙂

@hostep
Copy link
Member

hostep commented Aug 2, 2020

#11 is fixed. Released v1.2.0

Thanks again!

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

2 participants