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

Split out patch resolution into Composer Capabilities #212

Merged
merged 17 commits into from
Jun 2, 2018

Conversation

cweagans
Copy link
Owner

@cweagans cweagans commented May 18, 2018

This enables users to extend patch resolution in their own plugin. This would allow someone to essentially define their own logic for finding patches to apply to a package.

A lot of this code was ported out of the 2.x-prototype branch.

Remaining tasks:

  • Get checkPatches() working again. Since we're now resolving patches before the first package is installed/updated, checkPatches() doesn't have any of that data. The obvious solution is to run patch resolution earlier, but that might be tricky.
  • Dependency patch resolution.
  • Dependency patch resolution opt-out flag. Alternatively (or maybe in addition), some way to disable specific patch resolvers.
  • Test coverage

@cweagans cweagans force-pushed the resolver-capabilities branch from 1c1d287 to 4a946db Compare May 18, 2018 05:16
Repository owner deleted a comment from coveralls May 18, 2018
@cweagans
Copy link
Owner Author

The fact that the tests are currently passing is not a good thing. Shows how inadequate the test coverage is currently 😢

@cweagans
Copy link
Owner Author

cweagans commented Jun 2, 2018

The coveralls error is interesting. This PR increases coverage from ~7% to ~32% if I'm not mistaken.

This was referenced Jun 2, 2018
$patches = json_decode($patches, true);

// First, check for JSON syntax issues.
$error = json_last_error();

Choose a reason for hiding this comment

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

Seems like you could use php's json_last_error_msg() and skip this whole switch/case statement below.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good call. The current plugin has this big switch/case statement because it supports earlier versions of PHP. I'm only planning on supporting 5.6+ for 2.0.0, so json_last_error_msg() sounds great :) Thanks for catching that!

@cweagans cweagans merged commit f17c40d into master Jun 2, 2018
@cweagans cweagans deleted the resolver-capabilities branch June 2, 2018 22:32
@mbrodala mbrodala mentioned this pull request Oct 25, 2024
3 tasks
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