Do not delete local.patch file after reapplying

[kasperg]

bandaid-apply checks for the existence of a local.patch file and applies it if available. Afterwards the patch file is deleted. Is this necessary?

In my opinion the existence of a local.path file shows developers that the project has been modified locally and that these changes should be reapplied after updating the project resulting in the following workflow:

1. `drush dl some-project``
2. If a some-project.local.path file exists: drush ba some-project.

If the local.patch file is deleted the following workflow seems to be required

1. drush bt some-project - or check VCS history to see if the files for the project have been changed after updating
2. `drush dl some-project``
3. If a some-project.local.path file exists: drush ba some-project.

[xendk]

I don't like the idea of running ba using a local.patch that was committed to the repo at some time. Using bt assures that you'll get all the changes that's been applied to a project, even if someone slipped up and accidentally committed something without updating the patch.

I'd rather do a bd to check for local changes before updating modules, rather than relying on the previous developer having run bt/ba and committed the patch file.

For projects that's been patched and have a yml file, that should be reason enough to use bt/ba to reapply "proper" patches rather that reapplying by hand, which then saves any local changes. That leaves the case of projects that's only been locally modified to deal with.

[kallehauge]

I would agree with @xendk . The risk of someone using a outdated local.patch file kind of make every other argument mute. An alternative could be a separate file like a "readme" which could contain info about the latest bt. But I do not like the idea of keeping the local.patch file.

[kasperg]

In my opinion an outdated - but applicable - patch is worse than no patch.

If no patch is applied then none of the goals of patching the project are achieved.
If the patch no longer works as expected then chances are it no longer applies.
If the patch is outdated but applies then at least some of the goals of patching the project are achieved.

To sum things up I think the problem of the current approach is that introduces more work for the developer updating the project than the developer modifying the project. It should be the other way around given that updating projects is a standard part of Drupal development. Modifying projects is an exception - especially when it does not involve a public patch.

[kasperg]

Nomatter where the information regarding all local changes are stored the experience provided by drush-patchfile is admirable:

drush dl noderefcreate
Install location sites/all/modules/contrib/noderefcreate already exists. Do you want to overwrite it? (y/n): y
Project noderefcreate (7.x-1.0) downloaded to sites/all/modules/contrib/noderefcreate. [success]
noderefcreate patched with 763454-9.patch. [ok]

[xendk]

Re the automatic repatching of projects, yes, that is nice, and I've been thinking of stealing that idea. The thing is, drush-patchfile will lose local modifications quietly, while implementing bt and ba around drush dl will keep them (or at least cause it to fail). We might not like local modifications, but assuming it never happens is dangerous.

Good point on putting the burden of local modifications on the modifier rather than the updater. But currently bandaid does a pretty good job of helping out the latter even when the former have pissed on the project.

Might be that we're not quite aligned on what bandaid tries to be. Currently it tries to save your arse, even if you've wet the bed by making local modifications. It can also be used to try to pick patches apart on code someone else messed up, if you can guess some of the patches applied.

In other words, it's pragmatic. We'd rather not have local modifications, but it'll try to deal with them anyway.

What I'd suggest would be to hook into drush dl so we'll always do a bt before updating (will need some magic for performance, but I think it's doable), and change bt to, per default, to ask whether it should stash the local modifications into a file somewhere and make the yaml file point to it, to make the changes explicit. And a bandaid-save-patch (or something) that does the same, for when you just hacked the module and want to commit it properly.

Actually, I think the "take the local changes and stuff them into a patch file referenced by the yaml" command will go a long way in eliminating local modifications.