First feedback for v2 #484
Replies: 4 comments 1 reply
-
Thank you for the feedback!
#494 addresses the
Opened #495
Not sure I agree here. The default patches file in the new plugin is
This is intentional. If a user specifies that a patch should be applied, I'm not sure it's valid to just ignore when that patch can't be applied.
https://docs.cweagans.net/composer-patches/usage/recommended-workflows/#add-a-patch-to-your-project Yep. In 2.x, you have to be a bit more explicit sometimes. In 1.x, people were having problems with the plugin just randomly deleting directories full of code (some of which may have been work-in-progress). |
Beta Was this translation helpful? Give feedback.
-
Hmm, that doesn't sound right. Imagine, someone has a typo in their configuration and won't notice for months, just because nothing ever complained about it? I see your point re. the default value, but that could be handled by not complaining if the value equals the default and that isn't there. In all other cases, a missing file should trigger an error.
It was convenient in the past, as such "errors" most often occurred when the patch was merged by the package maintainer. In that case, we saw the list of not applying patches and then reviewed them all at once to correct the complete list of patches. With the new model, which makes perfect sense in an ideal world, we will only get a chance to notice the first failing patch, fix that and then run the update again. That has to go on until all of them have been identified. In the past, wasn't there a command line argument to switch to this strict mode as well? If so, wouldn't it make sense to no have such a switch with the reverse functionality, i.e. fail by default but allow to overwrite that e.g. in dev environments?
That makes sense, we've got to adopt to that behaviour and I guess the change will then have to be even bigger, especially in deployment pipelines. I guess we then need to go as far as this:
Not sure, if that's even possible !? |
Beta Was this translation helpful? Give feedback.
-
I'm open to putting a warning in the output if the patches file is missing.
This is what you have to do with Composer if e.g. you have an incompatible set of packages or you're missing platform deps. I'm open to a switch to make it not fail.
I don't understand why you would need to do that. Just |
Beta Was this translation helpful? Give feedback.
-
@cweagans Hi, |
Beta Was this translation helpful? Give feedback.
-
Found a couple of issues when giving
dev-main
a first try:extras/composer-patches
, they need to go into extras directly. Otherwise, they don't do anything.composer update
. With the new patches.lock file, do we have to force the repatching now manually, i.e. first relock the patch file and then repatch?Beta Was this translation helpful? Give feedback.
All reactions