Fix image style generation for d7 #2501
Conversation
| $extra = ' "' . __DIR__ . '/d8-rs-router.php"'; | ||
| } | ||
| elseif (drush_drupal_major_version() == 7) { | ||
| $extra = ' "' . __DIR__ . '/d7-rs-router.php"'; | ||
| } |
There was a problem hiding this comment.
This looks fine in terms of functionality. The only feedback I would have would be if it would be worth considering using the core "engines" hooks to manage this, rather than a conditional include - see https://github.com/drush-ops/drush/blob/master/includes/engines.inc for details. This would be part of the "drupal" engine type, which would provide automatic selection of the include based on the core version - similar to the "pm" engine you can see defined in https://github.com/drush-ops/drush/blob/master/commands/core/core.drush.inc#L33 and https://github.com/drush-ops/drush/tree/master/commands/core/drupal. Apart from organization and making the engines available to other commands (that do rs-type stuff), this could allow the engines to supplement the rs command docs (if they provide additional options etc).
There was a problem hiding this comment.
Given that the current code is already doing a conditional, this PR isn't making the situation any worse. I'd propose putting off improving the organization to a follow-up PR after fixing the bug.
|
I tested this PR with the Panopoly test suite on Travis-CI, and this got the image styles generating correctly! 👍 |
|
Huzzah! Thanks, Everyone :-) |
This is an update to the PR in #1658
I've split d8 and d7 code into separate files. The PR is against 8.x because master no longer supports Drupal 7.