drupal-boilerplate icon indicating copy to clipboard operation
drupal-boilerplate copied to clipboard

Document intent of Patches directory to avoid confusion

Open davereid opened this issue 11 years ago • 11 comments

This may be what I'm used to, but when all the patches are provided in the patches directory, when I go to actually update modules, themes, or core, I find that it takes me longer to figure out if a patch was applied or not.

What i'm used to:

  • Place patch in the root directory of the project being patched (for core, patches go in /docroot, for modules, patches go in the module directory i.e. /docroot/sites/all/modules/contrib/mymodule).
  • When I do a drush dl mymodule to download the new update, I can clearly see in the /docroot/sites/all/modules/contrib/mymodule directory that a patch file has gone missing and that I likely need to re-apply it.

I propose getting rid of the /patches directory.

davereid avatar Jan 15 '14 22:01 davereid

@davereid, on the other hand, with the current set up you look for the patches directory to see if there is a patch for a certain module. It also has the benefit of keeping patches out of the docroot.

juampynr avatar Jan 16 '14 11:01 juampynr

Our pattern is to have a running README.md file in the patches directory containing a maintained list of patch files for anything in the project, along with a link to the issue where the patchfile is created. This md file is grouped by project and sorted alphabetically, so that multiple patches to a single project are essentially grouped together. I'm going to re-open this issue, because if this isn't already documented in that directory, it needs to be.

Anything in boilerplate is optional, so if you have a project that you would not like to follow this pattern, you are of course free to do so.

q0rban avatar Jan 16 '14 13:01 q0rban

The README.md is documented, but I guess my confusion is the projects I have encountered so far have not had a maintained README.md of the patches. This has been confusing to me because I have to fall back to scanning the patch filesnames, or even the individual patch content to see if it applies to the module I care about.

Might be worth updating the boilerplate README.md to the most current format if it has changed in practice.

I'm just so used to doing a drush dl modulename and then already seeing in the diff "hey a patch was removed, I should investigate that!" which was nice. But it's not a one-true workflow by any means. I wonder if I could add symlinks to the patches in /patches to the directory of the thing their patching. If that creates more problems then it's worth, I won't do that.

davereid avatar Jan 16 '14 14:01 davereid

I'll suggest another alternative - projects should always have a make file if possible. Make files are great at documenting patches in a standardized format (You must define which module it applies to, can use comments to describe why the patch needs to be applied).

davereid avatar Jan 16 '14 16:01 davereid

I like being able to see it in the diff. If we could get the patch list in a machine readable format, we could have a drush post callback for DL that would inform you of the patches for a project after a new version is downloaded.

On Thursday, January 16, 2014, Dave Reid [email protected] wrote:

The README.md is documented, but I guess my confusion is the projects I have encountered so far have not had a maintained README.md of the patches. This has been confusing to me because I have to fall back to scanning the patch filesnames, or even the individual patch content to see if it applies to the module I care about.

Might be worth updating the boilerplate README.md to the most current format if it has changed in practice.

I'm just so used to doing a drush dl modulename and then already seeing in the diff "hey a patch was removed, I should investigate that!" which was nice. But it's not a one-true workflow by any means. I wonder if I could add symlinks to the patches in /patches to the directory of the thing their patching. If that creates more problems then it's worth, I won't do that.

— Reply to this email directly or view it on GitHubhttps://github.com/Lullabot/drupal-boilerplate/issues/22#issuecomment-32471113 .

James Sansbury | Development Manager | Lullabot o: (877) 585-5226 x 722 e: [email protected] t: twitter.com/q0rban w: lbt.me/q0rban

q0rban avatar Jan 16 '14 16:01 q0rban

Maybe something like a patches.make file?

; Add Migrate support for redirects.
; @see https://drupal.org/comment/5699446#comment-5699446
projects[redirect][patch][] = "https://drupal.org/files/1116408_26_migrate_redirect.patch"

This is machine-readable and could easily be parsed by Drush. Frankly it's easy for me to read/scan as well. Also if the project is actually using make files, it can easily be included from the primary make file:

includes[] = "patches.make"

davereid avatar Jan 16 '14 16:01 davereid

The ease of parsing a make file could also be used to keep a nicely formated markdown README in the patches directory up to date.

:+1:

blakehall avatar Jan 16 '14 16:01 blakehall

:+1:

However, is this meant to be used by drush? I think we just look at it as a quick way to see if something is patched.

juampynr avatar Jan 16 '14 16:01 juampynr

The patches.make approach seems to have merit and agreement, yay. I can make start here by providing an empty file, documentation for what to add, and integrate it with Drush to notify when patches need to be re-checked.

davereid avatar Jan 16 '14 16:01 davereid

I'll suggest another alternative - projects should always have a make file if possible. Make files are great at documenting patches in a standardized format (You must define which module it applies to, can use comments to describe why the patch needs to be applied).

I was a fan of this as well, but when we discussed this back in the day, the consensus was that most people preferred the patches directory, and to not maintain a drush make file that would never get used.

James Sansbury | Development Manager | Lullabot o: (877) 585-5226 x 722 e: [email protected] t: twitter.com/q0rban w: lbt.me/q0rban

q0rban avatar Jan 16 '14 16:01 q0rban

With the move to Drupal 8, I would deprecate having a drush makefile and favor using Composer instead.

sheldonrampton avatar Jun 14 '15 17:06 sheldonrampton