Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TASK] Only read override settings file if it has config #52

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

schloram
Copy link
Contributor

@schloram schloram commented Jul 20, 2023

In our setup we don't use the defaultConfiguration step but use the use the SetupConfigurationAction to set customOverrideSettings.

Because of the missing defaultConfiguration step the override.settings.yaml was not created before the SetupConfigurationAction got executed. In this case readConfig() throws an error that the file does not exist.

Therefore we have to check if the file has config before reading it.

In out setup we don't use the `defaultConfiguration` step but use the use the `SetupConfigurationAction` to set `customOverrideSettings`.

Because of the missing `defaultConfiguration` step the `override.settings.yaml` was not created before the `SetupConfigurationAction` got executed. In this case `readConfig()` throws an error that the file does not exist.

Therefore we have to check if the file has config before reading it.
@helhum
Copy link
Owner

helhum commented Feb 7, 2024

In our setup we don't use the defaultConfiguration step but use the use the SetupConfigurationAction to set customOverrideSettings.

Hm, I won't recommend that. The DefaultConfiguration action in TYPO3 core does way more than the name suggests. I therefore would not skip that, unless you have some test scenario or replicate the steps done there within a custom action.

@helhum
Copy link
Owner

helhum commented Feb 7, 2024

Your code change is fine though. It does not harm to check the overrides file exists before modifying it.

@helhum helhum merged commit 785883c into helhum:main Feb 7, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants