Make default bundle paths in node-renderer and Rails consistent#399
Make default bundle paths in node-renderer and Rails consistent#399
Conversation
bc73283 to
45936e8
Compare
| allWorkersRestartInterval: (env.CI && 2) || env.RENDERER_ALL_WORKERS_RESTART_INTERVAL || 10, | ||
| allWorkersRestartInterval: env.CI ? 2 : (env.RENDERER_ALL_WORKERS_RESTART_INTERVAL || 10), | ||
|
|
||
| // time in minutes between each worker restarting when restarting all workers | ||
| delayBetweenIndividualWorkerRestarts: (env.CI && 0.01) || 1, | ||
| delayBetweenIndividualWorkerRestarts: env.CI ? 0.01 : 1, |
There was a problem hiding this comment.
Just to improve readability.
| delayBetweenIndividualWorkerRestarts: (env.CI && 0.01) || 1, | ||
| delayBetweenIndividualWorkerRestarts: env.CI ? 0.01 : 1, | ||
|
|
||
| // Uncomment and change value for testing the honeybadger API integration |
There was a problem hiding this comment.
Nothing to uncomment.
45936e8 to
d2f83e9
Compare
|
|
||
| // time in minutes between restarting all workers | ||
| allWorkersRestartInterval: (env.CI && 2) || env.RENDERER_ALL_WORKERS_RESTART_INTERVAL || 10, | ||
| allWorkersRestartInterval: (env.CI ? 2 : env.RENDERER_ALL_WORKERS_RESTART_INTERVAL) || 10, |
There was a problem hiding this comment.
(env.RENDERER_ALL_WORKERS_RESTART_INTERVAL || 10) would actually make more sense, but Prettier removes the parentheses and in my opinion having no parentheses at all is less readable.
|
|
||
| // Find the .node-renderer-bundles folder if it exists, otherwise use /tmp | ||
| function defaultBundlePath() { | ||
| let currentDir = process.cwd(); |
There was a problem hiding this comment.
Maybe we should start from __dirname instead? But I'd expect process.cwd() to usually be the Rails root in practice, so this makes the search quicker.
There was a problem hiding this comment.
you can't use __dirname unless you want to depend on where node_modules is.
There was a problem hiding this comment.
Wouldn't it still usually be somewhere under Rails.root (in the cases we're on the same machine)?
There was a problem hiding this comment.
how do you get Rails.root in Node
There was a problem hiding this comment.
I mean, node-renderer will either be under node_modules in the Rails root, or something like node-renderer-stand-alone in Popmenu, also under Rails root. In both cases going up from __dirname should work.
|
|
||
| // Find the .node-renderer-bundles folder if it exists, otherwise use /tmp | ||
| function defaultBundlePath() { | ||
| let currentDir = process.cwd(); |
There was a problem hiding this comment.
you can't use __dirname unless you want to depend on where node_modules is.
See
react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb
Line 20 in c872880