Distribute mamba with workbench #1604
Conversation
Fix regular expression for env match. Add flag to quiet Windows cmd pop ups
There was a problem hiding this comment.
Thanks @emlys and sorry for being so slow. I had a few comments and questions but nothing major.
You mentioned commenting out the puppeteer test, I don't see that in here. Was that testing pulled out of this PR or handled elsewhere?
workbench/src/main/findBinaries.js
Outdated
| * @returns {string} mamba executable. | ||
| */ | ||
| export function findMambaExecutable(isDevMode) { | ||
| // Binding to the invest server binary: |
workbench/src/main/findBinaries.js
Outdated
| const ext = (process.platform === 'win32') ? '.exe' : ''; | ||
| const filename = `invest${ext}`; |
There was a problem hiding this comment.
It looks like these are leftover and not used.
workbench/src/main/main.js
Outdated
| const investExe = findInvestBinaries(ELECTRON_DEV_MODE); | ||
| settingsStore.set('investExe', investExe); | ||
| settingsStore.set('mamba', findMambaExecutable(ELECTRON_DEV_MODE)); |
There was a problem hiding this comment.
This is nitpicky but do we want to be consistent with whether we create a variable for a returned binary path or just call it from within the function signature?
|
@dcdenu4 I fixed those small issues. I had found that the puppeteer test failure was not specific to this branch, but was also happening on |
PR to merge the mamba distribution work into the plugins feature branch.
Description
Fixes #
Checklist