There's room for improvement in hook_uninstall():

  • Use Drupal's API to handle removing directories.
  • Remove files left behind in configuration migrations.
  • Delete all variables defined by the module.
CommentFileSizeAuthor
#1 garbage-collecting-1653094-1.patch1.63 KBhelior

Comments

helior’s picture

Status: Active » Needs review
StatusFileSize
new1.63 KB
dragonwize’s picture

Status: Needs review » Needs work

The gotcha here that needs more thinking is if configuration module is being used correctly then the config files and dir will be non-writable by drupal in production. This is why we also have a download option to backport config from prod in cases where DB changes were made there and need to be backported.

The correct way would be that the changes would not be made on live and only made on dev then committed and pushed to live. However, as much as we try, those situations will happen with the vast general public and even ourselves if we are not careful or get into a situation.

I think it might be good enough to just make a check that the dir is writeable, maybe even on the module level not just uninstall as this info is good in other situations. That way it is possible to uninstall the module without removing the config and instead of the user getting PHP errors they should just get a notice from us saying we were unable to remove the files.

dagmar’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Needs work » Active