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

Update phpunit and wp-phpunit packages #102

Merged
merged 6 commits into from
Jun 17, 2022

Conversation

shadyvb
Copy link
Contributor

@shadyvb shadyvb commented Jun 15, 2022

Ref humanmade/product-dev#1034

@shadyvb shadyvb force-pushed the product-dev/1034/update-phpunit branch from cae6cd1 to b0d4a7e Compare June 15, 2022 07:04
@shadyvb
Copy link
Contributor Author

shadyvb commented Jun 15, 2022

@roborourke I kept running into JS dep conflicts, so opted for using --legacy-peer-deps with npm install, things seem to work, however not entirely sure about it. Tried to resolve it with no luck. Thoughts ?

@shadyvb shadyvb requested a review from roborourke June 15, 2022 07:08
Copy link
Contributor

@roborourke roborourke left a comment

Choose a reason for hiding this comment

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

Some of the composer.json edits don't make sense, and I'm not super clear on why any JS changes were necessary but if the dev server etc works fine still then I don't see any problem. If the GH action is using ubuntu-latest and we're not specifying the node version that might be the issue. The node version should always be pinned, and preferably the ubuntu version too to keep a consistent dev environment.

composer.json Outdated
"roots/wordpress": "~5.9.0",
"squizlabs/php_codesniffer": "3.5.8",
"szepeviktor/phpstan-wordpress": "0.7.1",
"vlucas/phpdotenv": "^3",
"wp-cli/db-command": "^2",
"wp-phpunit/wp-phpunit": "~5.9.0",
"wp-phpunit/wp-phpunit": "5.9.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

This constraint should probably match roots/wordpress, I'm not sure why this has changed, the result is the same as the previous constraint but is now locked to that specific version.

Worth bumping to WP 6.0 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, the version constraint was not intentional, should've updated but kept the tilda. Sorry. And yes we should probably upgrade to 6. Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to update that package explicitly, but just forgot to add the tilda in the command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm opting to keep on 5.9.3 until WP 6.0 is properly tested. Fixed the tilda.

@shadyvb shadyvb requested a review from roborourke June 17, 2022 09:28
@shadyvb shadyvb merged commit 667e54d into develop Jun 17, 2022
@shadyvb shadyvb deleted the product-dev/1034/update-phpunit branch June 17, 2022 09:38
@kovshenin kovshenin mentioned this pull request Jul 14, 2022
kovshenin added a commit that referenced this pull request Jul 14, 2022
This avoids installing or attempting to resolve peer dependencies.

See #102 (comment)
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.

2 participants