-
Notifications
You must be signed in to change notification settings - Fork 98
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
Global settings override with user-specific json file #131
Comments
@purcebr Interesting idea. To what extent would your data.json files differ? |
@RyanHavoc for the current setup, we're using different "local" copies of the css file. So, each person is simultaneously working on the same sass toolchain independently, and each have a different url for their respective generated css file. This might be pretty specific to our use case, but I'd be curious to hear what you think! |
@purcebr So you're all working on the same CSS but the generated filenames are different for each person? I am thinking about the best way to allow for environments with Astrum, typically this is for say local, staging, production environments but it would work in your case as well. I'm thinking there'd be the default data.json as it is, but also the option for an assets.json which if present overrides the assets array in data.json. assets.json can then be set to be ignored in Git so different people/environments can use it to load assets specific to them. |
yeah, absolutely! that would be great. |
Since it's not required, though, I wonder if there would be a better way to check for it than sending a get request? I seems unideal to depend on the 400 level response to see if the environment file is there. |
@RyanHavoc mind if I take a stab at a PR for this update? |
@purcebr Sure. We'll need to replicate whatever you come up with for release/2.0.0 though, or just start from release/2.0.0 as I'm trying to only push bug fixes to master. All the new stuff is going in release/2.0.0. |
curious - Is there another way to conditionally pull in the assets.json file, instead of trying to GET it and checking for a 404? is that the solution you were thinking of? another option could be to add a flag to check for the custom assets file in the data.json, but that's got it's own drawbacks in terms of potential for confusion and general non-userfriendlyness |
@purcebr Astrum can make dozens of requests as it's "building" the pattern library, as it's loading in all the component files and markdown files. I don't think one extra to check for an assets.json file is much of an issue. I'd rather than than having to remember to include a flag. |
@RyanHavoc custom styles and scripts should ADD to, not replace the styles and scripts in the data.json, right? |
@purcebr I'd have them override entirely. Less likelihood of unexpected style conflicts and feels more "environment" like. |
👍 |
@RyanHavoc why not allow them to overwrite any top level group, and call the file "custom.json" instead? that way, the custom json structure would mirror the structure of the data.json, so people could use the data.json as a starting point for their environmental overrides. |
@purcebr I thought about this but beyond the assets what other variables would you want to change? My concern is that the data.json file is actively modified by Astrum as you're adding components. We don't want to have to maintain two files for no reason... |
@RyanHavoc That pr should cover the assets.json file override. Let me know how that looks! |
I'll check out 2.0 and send over a patch for that branch, as well. |
Hey all, I've picked this up now. I'm going to do a fully overridable I'll call the local file Thanks for the PR @purcebr, it's great work. The main reason we're not doing just assets now is that the |
Hi! - We're using your tool across several developers, and were interested in possibly implementing a feature that would allow individuals to override the global settings in the data.json file with their own settings. That way, they could have a config that's not checked into git.
Is this something you'd be interested in integrating, If I sent over a PR?
thanks!
Bryan
The text was updated successfully, but these errors were encountered: