Split config #578

ralphtheninja · 2019-01-10T19:42:04Z

Ticks off final checkbox in #550

Simon-Laux · 2019-01-10T19:56:14Z

When we're already at it, should we add height and width arguments to rc to add like a --window-imensions or --win-dim [widht]x[height] argument? It might be useful for some users

ralphtheninja · 2019-01-10T20:34:39Z

src/main/windows/main.js

@@ -176,12 +180,6 @@ function toggleDevTools () {
  }
 }

-function getIconPath () {
-  return process.platform === 'win32'
-    ? config.APP_ICON + '.ico'


Broken. Lets fix this when we need .ico file for windows.

I made a TODO comment about it in application-constants.js.

ralphtheninja · 2019-01-10T20:35:09Z

src/renderer/lib/state.js

 const log = require('../../logger').getLogger('renderer/state')

 const SAVE_DEBOUNCE_INTERVAL = 1000

-appConfig.filePath = path.join(config.CONFIG_PATH, 'config.json')


Not needed, appConfig.filePath defaults to /path/to/config.json

ralphtheninja · 2019-01-10T20:37:50Z

When we're already at it, should we add height and width arguments to rc to add like a --window-imensions or --win-dim [widht]x[height] argument? It might be useful for some users

Basic rule of thumb: Never add something that might be useful. We should add default values to rc though for the log stuff. Default is undefined at the moment and it's good practice to set e.g. false instead.

ralphtheninja · 2019-01-11T08:24:30Z

I just realized that "splitting" here basically meant renaming config.js to application-constants.js and with a functional API instead of passing around an object. However, we cleaned up some stuff that wasn't needed / used in the process.

src/main/index.js

src/main/windows/main.js

src/renderer/components/dialogs/About.js

test/integration/setup.js

ralphtheninja added 9 commits January 10, 2019 15:11

Move APP_ICON to application constants

eac5ee5

Move APP_{NAME,VERSION,WINDOW_TITLE} to application constants

a62974e

Remove GITHUB_URL_RAW, not used

262a072

Move GITHUB_URL, GITHUB_URL_ISSUES, HOME_PAGE_URL + add url for license

868e83b

Remove UI_MESSAGE_HEIGHT, not used

8082a66

Remove dom-ready event handler, not used

04125c6

Move window defaults to application constants

ee5064c

Remove STATIC_PATH, not used

0069d52

Move APP_CONFIG to application constants

47b82fa

ralphtheninja requested review from Simon-Laux and Jikstra January 10, 2019 19:42

ralphtheninja commented Jan 10, 2019

View reviewed changes

Fix standard

0a38a14