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

Reconsider to fail the build by default when patches cannot be applied #209

Closed
pfrenssen opened this issue May 4, 2018 · 1 comment
Closed

Comments

@pfrenssen
Copy link
Contributor

In #40 the functionality was added to optionally allow a build to fail if a patch cannot be applied by setting the composer-exit-on-patch-failure option.

At the time it was decided that this option would not be made default to preserve backwards compatibility, and that in an upcoming 2.x release it would be made the default.

I would like to propose to make this the default anyway. It looks like a 2.x version is going to be a long time off, and in the meantime we keep getting hit by this. It's impossible to imagine the number of hours that were lost by developers looking into failed tests because a patch didn't apply any more, and it was forgotten to set this option. I have personally been hit by this more times than I would like to admit.

I think it depends on your viewpoint: this could be considered as a feature that breaks backwards compatibility, or it could be seen as a bugfix for previously unintended behaviour, with a flag to preserve backwards compatibility.

From my point of view, if I define a patch in my composer.json file, I am declaring that my project depends on this patch, I don't consider it to be optional.

An alternative would be to make a 2.0 release in the near future, but that seems excessive for just this feature alone.

@cweagans
Copy link
Owner

cweagans commented Jun 2, 2018

This is squarely 2.0.0 material as it's a breaking change in behavior. While I agree it's the right behavior, I'm not willing to shove it in to 1.x and hope for the best. It's already in 2.x, and the best way to get it out the door is to help me with #93 (will be updated within an hour or so with 2.0.0 hitlist - working through issue triage right now).

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

No branches or pull requests

2 participants