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

Composer: Upgrade deps for composer 2 #47498

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Composer: Upgrade deps for composer 2 #47498

merged 3 commits into from
Nov 27, 2020

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Nov 17, 2020

Upgrade composer dependencies.

I have composer 2 installed, one of our composer packages is incompatible with it. Upgrade that composer package and upgrade our composer deps.

Previously, I was hitting issues with yarn dev from apps/editing-toolkit-plugin:

[dev:newspack-blocks               ] done
[dev:newspack-blocks               ] phpcbf: ./bin/sync-newspack-blocks.sh: line 132: ../../vendor/bin/phpcbf: No such file or directory
[dev:newspack-blocks               ] !! There was an error executing phpcbf!

I could not composer install becuase:

Your lock file does not contain a compatible set of packages. Please run composer update.

  Problem 1
    - dealerdirect/phpcodesniffer-composer-installer is locked to version v0.6.2 and an update of this package was not requested.
    - dealerdirect/phpcodesniffer-composer-installer v0.6.2 requires composer-plugin-api ^1.0 -> found composer-plugin-api[2.0.0] but it does not match the constraint.

Testing instructions

I'm not sure how many places use composer, but yarn dev under apps/editing-toolkit-plugin should work after this change.

@matticbot
Copy link
Contributor

@sirreal sirreal marked this pull request as ready for review November 17, 2020 12:28
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 17, 2020
@sirreal sirreal added dependencies Pull requests that update a dependency file Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin labels Nov 17, 2020
@sirreal sirreal requested a review from a team November 17, 2020 12:28
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@noahtallen
Copy link
Contributor

I pushed a tiny readme change to the editing toolkit plugin. That way we can see if the CI tools which rely on this package work! If that passes, this looks great to me

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D52895-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

Copy link
Contributor

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@noahtallen
Copy link
Contributor

Note: the phpunit failure is a separate issue

@ockham
Copy link
Contributor

ockham commented Nov 17, 2020

Thanks for reviewing, Noah!

I pushed a tiny readme change to the editing toolkit plugin. That way we can see if the CI tools which rely on this package work! If that passes, this looks great to me

Shall we revert that change, or keep it? 😄

@noahtallen
Copy link
Contributor

Shall we revert that change, or keep it? 😄

we can keep it, just a tiny grammar/style improvement 😁

@sirreal sirreal added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 18, 2020
@sirreal sirreal force-pushed the update/composer-deps branch 2 times, most recently from e2f280a to 41d77be Compare November 20, 2020 12:41
@sirreal
Copy link
Member Author

sirreal commented Nov 20, 2020

Note: the phpunit failure is a separate issue

So this is safe to ignore, @noahtallen?

Shall we revert that change, or keep it? 😄

we can keep it, just a tiny grammar/style improvement 😁

Noting that I have no intention of releasing Editing Toolkit for this markdown change, but this should be completely safe to release with other changes.

@sarayourfriend sarayourfriend changed the base branch from master to trunk November 20, 2020 16:10
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 20, 2020
@noahtallen
Copy link
Contributor

Noting that I have no intention of releasing Editing Toolkit for this markdown change, but this should be completely safe to release with other changes.

Absolutely, I would not have expected you to :)

So this is safe to ignore

Yes, it would have been. I believe the issue has been fixed over the past several days, so after a rebase I would expect it to pass now.

@sirreal sirreal force-pushed the update/composer-deps branch from 41d77be to baa9498 Compare November 24, 2020 20:28
@sirreal
Copy link
Member Author

sirreal commented Nov 24, 2020

Rebased.

Noting that I have no intention of releasing Editing Toolkit for this markdown change, but this should be completely safe to release with other changes.

Absolutely, I would not have expected you to :)

That was for posterity or someone handling a release, didn't intend to direct it at you 😆

@sirreal
Copy link
Member Author

sirreal commented Nov 25, 2020

Attempting to fix the tests: #47766

@sirreal
Copy link
Member Author

sirreal commented Nov 27, 2020

The phpunit tests are not a quick fix and are not introduced here. I'll merge.

@sirreal sirreal merged commit 36846a3 into trunk Nov 27, 2020
@sirreal sirreal deleted the update/composer-deps branch November 27, 2020 15:29
@matticbot matticbot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Ready to Merge labels Nov 27, 2020
@noahtallen
Copy link
Contributor

Phpunit tests fixed in #47979

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants