Skip to content

Use patches from GitHub to patch PHP-CSS-Parser #5556

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

Merged
merged 4 commits into from
Nov 2, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Nov 2, 2020

Summary

In GoogleForCreators/web-stories-wp#5082 @westonruter & @swissspidy discussed the idea of using patches from GitHub instead of providing ones locally. The advantage with this approach is that the remote patches are easier to update, and it would resolve the issue of other packages not being able to apply our local patches (see cweagans/composer-patches#315).

While working on this I found out the remote patch from MyIntervals/PHP-CSS-Parser#193 was not being applied cleanly, so I had to rebase with the latest changes from the master branch for it to succeed.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@pierlon pierlon added dependencies Pull requests that update a dependency file WS:Core Work stream for Plugin core labels Nov 2, 2020
@pierlon pierlon added this to the v2.0.6 milestone Nov 2, 2020
"Fix parsing CSS selectors which contain commas <https://github.com/westonruter/PHP-CSS-Parser/pull/1>": "patches/php-css-parser-commit-10a2501.patch"
"Add additional validation for size unit <https://github.com/sabberworm/PHP-CSS-Parser/pull/193>": "https://github.com/sabberworm/PHP-CSS-Parser/compare/3bc5ded67d77a52b81608cfc97f23b1bb0678e2f%5E...468da3441945e9c1bf402a3340b1d8326723f7d9.patch",
"Validate name-start code points for identifier <https://github.com/sabberworm/PHP-CSS-Parser/pull/185>": "https://github.com/sabberworm/PHP-CSS-Parser/compare/d42b64793f2edaffeb663c63e9de79069cdc0831%5E...113df5d55e94e21c6402021dfa959924941d4c29.patch",
"Fix parsing CSS selectors which contain commas <https://github.com/westonruter/PHP-CSS-Parser/pull/1>": "https://github.com/westonruter/PHP-CSS-Parser/compare/master...10a2501c119abafced3e4014aa3c0a3453a86f67.patch"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@westonruter I'm using master as the head for this patch because westonruter/PHP-CSS-Parser#1 includes a merge commit that would include a lot of unnecessary changes not relevant to the changes in the PR.

@pierlon
Copy link
Contributor Author

pierlon commented Nov 2, 2020

The lint Travis job is currently failing due to localheinz/composer-normalize dropping support for Composer v1. That should be fixed when #5554 is merged via fc5ddf2.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2020

Plugin builds for 2551ea8 are ready 🛎️!

@pierlon pierlon self-assigned this Nov 2, 2020
@pierlon
Copy link
Contributor Author

pierlon commented Nov 2, 2020

⚠️ Rebasing with latest changes from develop.

@pierlon pierlon force-pushed the enhancement/composer-remote-patches branch from 6eb83a7 to 2551ea8 Compare November 2, 2020 17:54
@westonruter westonruter merged commit 0985bfd into develop Nov 2, 2020
@westonruter westonruter deleted the enhancement/composer-remote-patches branch November 2, 2020 19:24
@westonruter
Copy link
Member

I switched to 2.0 branch, cherry-picked fc3514a and 3984655 and then ran composer update then squashed into 2950487.

@johnwatkins0 johnwatkins0 assigned johnwatkins0 and unassigned pierlon Nov 12, 2020
@johnwatkins0
Copy link
Contributor

@pierlon Any suggestions for QA testing this?

@johnwatkins0 johnwatkins0 removed their assignment Nov 12, 2020
@pierlon
Copy link
Contributor Author

pierlon commented Nov 12, 2020

Running composer install shouldn't result in an error. But that's already been proven since the command in the CI jobs has not been failing. This can be moved to "QA Passed".

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. dependencies Pull requests that update a dependency file WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants