fix: make datastore OS agnostic (path things) #13
Conversation
src/key.js
Outdated
| const pathSepS = path.sep | ||
| const pathSep = new Buffer(pathSepS, 'utf8')[0] | ||
| const pathSepS = '/' | ||
| const pathSepB = new Buffer(pathSepS, 'utf8') |
There was a problem hiding this comment.
needs to change to Buffer.from
There was a problem hiding this comment.
see the following comment
There was a problem hiding this comment.
What I'm referencing here is the linting issue https://travis-ci.org/ipfs/interface-datastore/jobs/296689734#L2153
|
|
||
| const Key = require('../src').Key | ||
|
|
||
| const pathSep = path.sep | ||
| const n = (p) => path.normalize(p) | ||
| const pathSep = '/' |
There was a problem hiding this comment.
If this is defined in multiple places, it would be better to have a src/constants.js file
There was a problem hiding this comment.
I'm not happy with the names, just dealing with the current code. You will note that pathSep is a Buffer in one and a string in another!
There was a problem hiding this comment.
You are welcome to change that :)
There was a problem hiding this comment.
@richardschneider you should be able to fetch this branch and push commits to it.
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
=========================================
Coverage ? 91.85%
=========================================
Files ? 5
Lines ? 393
Branches ? 0
=========================================
Hits ? 361
Misses ? 32
Partials ? 0
Continue to review full report at Codecov.
|
|
@diasdavid Is the codecov error a false positive? |
|
@richardschneider it is just because it is the first time it is running on this repo. It's fine :) |
|
@richardschneider released as 0.4.0. You can now update in the remaining PR's \o/ |
|
Cool, thanks for all help |
|
No, thank you for making it happen!! ❤️ |
|
Looks great guys, I'll take it for a spin today and let you know if I see any other trouble. |
No description provided.