- Issue created by @kim.pepper
- Merge request !12555[#3533291] Remove unused file_file_video_presave() β (Open) created by kim.pepper
- π¬π§United Kingdom catch
This isn't dead code as such, the upgrade path in that issue wasn't implemented correctly.
When we're updating configuration, we need a post update that updates configuration in the site database, but also a presave hook that updates config that is shipped with modules/install profiles/recipes and triggers deprecations.
See for example views_post_update_table_css_class()
And
/** * Implements hook_ENTITY_TYPE_presave(). */ #[Hook('view_presave')] public function viewPresave(ViewEntityInterface $view): void { /** @var \Drupal\views\ViewsConfigUpdater $config_updater */ $config_updater = \Drupal::classResolver(ViewsConfigUpdater::class); $config_updater->updateAll($view); }
π Add generic interface + base class for upgrade paths that require config changes Active would make it harder to get this wrong but for now we should fix this specific update to follow that pattern.
- π¬π§United Kingdom catch
Another possibility here is to ensure that the config is always valid with or without this config key. This is the current approach taken in π Make menu trail behaviour in SystemMenuBlock optional Active (see the config schema in the MR) which avoids having any update path at all for existing config - that issue isn't in yet but it might be viable for changes like this that are adding something brand new that defaults to off anyway.
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
So it's just adding `requiredKey: false` under the playsinline block in the schema?