You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've noticed that the theme app requires environment variables in order to properly function (at least locally), but the variables themselves are never used within this repo codebase at all. All the usages are actually in the theme-kit package.
I believe this breaks the separation of concerns principle. I cannot modify this theme to do something else with the env variables, or to instantiate the Prezly API client with my own local dev API endpoint.
It would be better if the outsourced theme-kit code was providing entry points expecting the required inputs (like tokens/env variables, or an instantiated API Client instance) and then performed its job.
Of course, this is by all means, not a critical problem. Just something rather good to have.
The text was updated successfully, but these errors were encountered:
I agree that this abstraction might not be the best in terms of intuitiveness, but it does help to reduce the boilerplate. The env variables needed to run the theme locally are documented both in the Theme Kit and the theme repos, so I don't see this as a big problem, at least for now.
I've noticed that the theme app requires environment variables in order to properly function (at least locally), but the variables themselves are never used within this repo codebase at all. All the usages are actually in the theme-kit package.
I believe this breaks the separation of concerns principle. I cannot modify this theme to do something else with the env variables, or to instantiate the Prezly API client with my own local dev API endpoint.
It would be better if the outsourced theme-kit code was providing entry points expecting the required inputs (like tokens/env variables, or an instantiated API Client instance) and then performed its job.
Of course, this is by all means, not a critical problem. Just something rather good to have.
The text was updated successfully, but these errors were encountered: