Skip to content
Snippets Groups Projects

Fix proposal for #3435910

Open Gaël Gosset requested to merge issue/config_split-3435910:3435910 into 2.0.x
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
931 string $name,
932 StorageInterface $sync,
933 StorageInterface $split,
934 StorageInterface $transforming
935 ): void {
936 if ($sync->exists($name)) {
937 // If a partial split has already been written, delete it.
938 if ($split->exists(self::SPLIT_PARTIAL_PREFIX . $name)) {
939 $split->delete(self::SPLIT_PARTIAL_PREFIX . $name);
940 }
941 /**
942 * On import, we will assume that an empty array config file means the
943 * config should not be imported (should be removed if it exists).
944 * @see \Drupal\config_split\ConfigSplitManager::mergeSplit()
945 */
946 $split->write($name, []);
  • I don't know if an empty array is a legitimate config. probably not. But I would much rather save the fact that the config should be deleted in some other way. Like writing ['config_split' => 'deleted config'] or something along the lines of that. This makes it much easier to see what is going on and we can make sure that this never reaches the active config because we know config split needs to deal with it. We do this for sequence keys already. Ie config split saves sequences with deterministic keys and then when merging the keys are stripped again.

  • or save a tombstone/artefact with a different name like the patches. that indicates that it should be removed. In a way that would be a patch that removes everything no? in that case it could have FALSE since that on a config read means it doesn't exist.

  • Gaël Gosset changed this line in version 4 of the diff

    changed this line in version 4 of the diff

  • I'm still not sure what the best structure would be. I made e259f419, then 49e5e6b6. Is it better like this? Note that createPatch wants an array as an argument, so I used [FALSE] and not FALSE. The tests still pass...

    Edited by Gaël Gosset
  • Please register or sign in to reply
  • Gaël Gosset added 1 commit

    added 1 commit

    Compare with previous version

  • Gaël Gosset added 3 commits

    added 3 commits

    • e259f419 - Use an array containing FALSE instead of an empty array to store the fact that...
    • d928a1ed - Fix the case where the config removed in the split was the only one existing in the collection
    • 5682557b - WIP: Tests for 3435910 (passing but incomplete)

    Compare with previous version

  • Gaël Gosset added 3 commits

    added 3 commits

    • 41baf783 - fixup! Use an array containing FALSE instead of an empty array to store the...
    • 0b7de760 - fixup! Fix proposal for #3435910 (the bug cannot happen for complete splits)
    • 15599482 - Tests for 3435910 (passing)

    Compare with previous version

  • Gaël Gosset marked this merge request as draft from issue/config_split-3435910@41baf783

    marked this merge request as draft from issue/config_split-3435910@41baf783

  • Gaël Gosset added 1 commit

    added 1 commit

    • 02e88cfb - Removed an unnecessary check

    Compare with previous version

  • Gaël Gosset added 1 commit

    added 1 commit

    • 05ce5630 - Removed an unnecessary check

    Compare with previous version

  • Gaël Gosset added 1 commit

    added 1 commit

    • 49e5e6b6 - Store removal as patch instead of a config file @see !47 (comment 301770)

    Compare with previous version

  • Gaël Gosset
  • Gaël Gosset
  • Gaël Gosset added 2 commits

    added 2 commits

    • 8020ff2c - Remove redundant tests on config_test.system
    • 99e032eb - Test for the case when the removed config was not the only one

    Compare with previous version

  • Gaël Gosset added 2 commits

    added 2 commits

    Compare with previous version

  • Gaël Gosset marked this merge request as ready

    marked this merge request as ready

  • Please register or sign in to reply
    Loading