Skip to content

Update composer dependencies and unit tests #61

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

Closed
wants to merge 1 commit into from

Conversation

drpayyne
Copy link

@drpayyne drpayyne commented Jun 26, 2021

Description

  • updated php to ^7.3||^8.0
  • updated phpunit/phpunit to ^9.5
  • updated nikic/php-parser to ^4.10
  • updated phpstan/phpdoc-parser to ^0.5
  • replaced zendframework/zend-stdlib with laminas/laminas-stdlib as Zend packages are deprecated
  • sorted composer dependencies :)
  • updated php to 7.3 in .github/workflows/php.yml

Fixed Issues

  1. Fixes Add PHP 8 compatibility to magento/semver magento2#32872

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@fascinosum
Copy link
Collaborator

hi @drpayyne,
thank you for your contribution.

Could you redirect it to the develop branch

You have a failed test build because PHP 7.2 is out of support. Could you update the PHP version used for the build? We need to run build with PHP 7.3

To properly test these changes, we need to create a temporary PR into magento2-infrastructure and update composer.json dependency to use your branch as a source. See an example
We need to run the SVC build with PHP 7.3 and PHP 7.4 for that PR
cc: @andrewbess

@drpayyne
Copy link
Author

Hi @fascinosum, I've updated PHP in composer and the GitHub workflow config file. Unfortunately I don't have access to the infra repo you have linked. I am part of a partner org (EY), so I'm not sure if that repo is available to partners.

Copy link
Contributor

@andrewbess andrewbess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @drpayyne
Thank you for your contribution.
I fixed the infrastructure and code styling problems.
Changes are approved.

Copy link
Contributor

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @drpayyne,
Could you review and fix the warnings when running unit tests?
You can find them here:
https://github.com/magento/magento-semver/pull/61/checks
image

@drpayyne
Copy link
Author

Sure @ihor-sviziev, I will fix those today.

composer.json Outdated
},
"require-dev": {
"phpunit/phpunit": "^6.5.0",
"phpunit/phpunit": "9.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we allow installing 9.3.x?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ihor-sviziev.
I changed it temporary to have some checks.
Now, I fixed all changes and squashed commits.

Copy link
Contributor

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update recent changes from the develop branch

- updated composer dependencies
- fixed build yaml file
@andrewbess
Copy link
Contributor

This pull request (PR) closed because has been created from wrong branch (master instead of develop) and has a lot of merge-conflicts.
New PR is #65

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.

Add PHP 8 compatibility to magento/semver
5 participants