System binary #119
System binary #119
Conversation
this also updates tests to use a mocked version of the MongoBinaryDownloader so that we have a truelly issolated unit test...
jtbairdsr
force-pushed
the
system-binary
branch
from
2a9d381
to
c0f2bf0
Compare
|
I have pushed changes in response to your comments #32 (comment). I look forward to your feedback... |
src/util/MongoBinary.js
Outdated
| } | ||
|
|
||
| debug(`MongoBinary: Mongod binary path: ${this.cache[version]}`); | ||
| return this.cache[version]; | ||
| if (version !== 'latest' && systemBinary) { |
There was a problem hiding this comment.
We should not show the message if versions are the same ;)
And I think that better to move this code inside if (systemBinary) { at line 149
right after binaryPath = await this.getSystemPath(systemBinary);
There was a problem hiding this comment.
All rest parts are good! Code became much cleaner! Thanks!
There was a problem hiding this comment.
Ouch! And don't forget to add new option to README.md
There was a problem hiding this comment.
Thanks, I appreciate the feedback and agree. I have updated the code and the README.md. let me know if there is anything else.
this is a fairly simple check and doesn't attempt to do any complex version matching or resolution. It simply takes the value provided by the system and checks strict equality with the version requested.
also added a section explaining how to use mongodb-memory-server on a system not officially supported by mongodb
src/util/MongoBinary.js
Outdated
| @@ -148,6 +148,21 @@ export default class MongoBinary { | |||
|
|
|||
| if (systemBinary) { | |||
| binaryPath = await this.getSystemPath(systemBinary); | |||
| const binaryVersion = execSync('mongod --version') | |||
There was a problem hiding this comment.
Should be
const binaryVersion = execSync(`${binaryPath} --version`)And need to add check that binaryPath is not empty ;)
There was a problem hiding this comment.
oh yeah, good catch totally missed that.
There was a problem hiding this comment.
I've just merged with the master (there was a conflict).
Pull changes before commit.
There was a problem hiding this comment.
There were updated packages in master.
Maybe it helps to resolve #120
There was a problem hiding this comment.
👌 I made that fix and pushed.
|
Thanks a lot! 👍 |
|
I'll fix |
|
No problem! It was a pleasure to participate. |
|
when will these changes be published to npm? |
|
🎉 This PR is included in version 2.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@nodkz thank you again for the awesome package! It was a pleasure to contribute! |
No description provided.