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

fix: scripts app_config #2432

Merged
merged 1 commit into from
Sep 20, 2024
Merged

fix: scripts app_config #2432

merged 1 commit into from
Sep 20, 2024

Conversation

chrismclarke
Copy link
Member

@chrismclarke chrismclarke commented Sep 20, 2024

PR Checklist

  • PR title descriptive (can be used in release notes)

Description

Fix regression introduced by #2423

Default app_config variables are no longer loaded until runtime, however there appears to be an instance where scripts that post-process assets expect to access the app_config theme variables. This would results in any deployments without themes defined being unable to compile

This fix simply adds fallback empty array for cases where deployment doesn't specify themes. An more robust solution should probably include improving type-definitions to asset config is only partial during script processing, and to move the theme config to the top-level deployment config - however likely not worth the time involved right now.

Git Issues

Closes #

Screenshots/Videos

If useful, provide screenshot or capture to highlight main changes

@chrismclarke
Copy link
Member Author

Tagging @jfmcquade so you are aware, but planning to merge as a hotfix as potential for knock-ons is minor. As well as I could check I couldn't see any other instances of scripts requiring values from default app config variables.

@chrismclarke chrismclarke merged commit f4c0307 into master Sep 20, 2024
4 checks passed
@chrismclarke chrismclarke deleted the hotfix/script-app-config branch September 20, 2024 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix scripts Work on backend scripts and devops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants