launchcodedev / app-config

Easy Configuration Loader with Strict Validation

https://app-config.dev

Mozilla Public License 2.0

69 stars 11 forks source link

Electron package #165

Closed johnb8 closed 3 years ago

johnb8 commented 3 years ago

Addresses #57. I couldn't implement it in the same way we do in our LC package since app-config doesn't have synchronous config loading anymore and Electron preload scripts don't wait for promises to resolve. Instead I load the config in the main electron process then pass that through to the renderer. I think this is better than the way we used to do it since the config is now available on the main electron side as well. I'm not sure if there's any concerns using the main config object across libraries like I do, but it works.

I'll add documentation and tests, just wanted to get your thoughts on how I've done this first.

joelgallant commented 3 years ago

@johnb8 seems to have timed out tests, not sure why

johnb8 commented 3 years ago

Yeah, not really sure what's going on, it doesn't seem to be running the electron tests at all? I added the playwright github action, but since it's electron it should have all the dependencies it needs so I'm not sure if it will help. I'm also not sure if it's a display thing either because I think Windows and MacOS should run it no problem without any display configuration. @joelgallant can you approve the run again?

johnb8 commented 3 years ago

Could you approve again @joelgallant?

codecov-commenter commented 3 years ago

Codecov Report

Merging #165 (7dc0548) into master (1792d22) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #165   +/-   ##
=======================================
  Coverage   80.09%   80.09%           
=======================================
  Files          42       42           
  Lines        2482     2482           
  Branches      593      593           
=======================================
  Hits         1988     1988           
  Misses        494      494

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1792d22...7dc0548. Read the comment docs.

johnb8 commented 3 years ago

@joelgallant are the Windows and MacOS pipelines actually running in those environments? If you look at the yarn playwright install-deps chromium step on them it says it's installing the Ubuntu dependencies and the paths are all Linuxy

johnb8 commented 3 years ago

@joelgallant CI is fixed, I think this is good to go now, unless you see anything else?