feat: Basic mfs.write command #1

achingbrain · 2018-05-02T12:31:36Z

Probably full of bugs.

coveralls · 2018-05-02T13:07:06Z

Pull Request Test Coverage Report for Build 44

0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
7 unchanged lines in 3 files lost coverage.
Overall coverage increased (+1.9%) to 94.077%

Files with Coverage Reduction	New Missed Lines	%
../src/core/utils/traverse-to.js	1	97.4%
../src/core/utils/add-link.js	1	75.0%
../src/core/write/index.js	5	88.06%

Totals
Change from base Build 28:	1.9%
Covered Lines:	732
Relevant Lines:	755

💛 - Coveralls

daviddias · 2018-05-03T14:14:20Z

circle.yml

@@ -1,7 +1,7 @@
 # Warning: This file is automatically synced from https://github.com/ipfs/ci-sync so if you want to change it, please change it there and ask someone to sync all repositories.
 machine:
  node:
-    version: stable


Please use LTS

If we're supposed to be standardising on Jenkins, do you think it might be better to remove the travis & circle setup instead?

✅ from me. cc @victorbjelkholm

Looks like the blocker on Jenkins is getting code coverage working.

Yeah, code coverage doesn't have a solution as of now. See ipfs-inactive/dev-team-enablement#23 (comment)

Once we have that in place, we can start removing Travis and Circle from everywhere.

Hmm, I changed it to be node version 8 and it's broken the Circle build. Which is odd because the travis build passes with node 8. Circle complains about leveldown being compiled for a later version of node - do we cache the dependencies between Circle builds?

@achingbrain the caching could be a bit funky in CircleCI somtimes, I've retried the job with cache reset this time. Running here: https://circleci.com/gh/ipfs/js-ipfs-mfs/34

Nice one, thanks - that appears to have fixed it.

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

victorb · 2018-05-08T16:49:25Z

package.json

    "test:node": "aegir test -t node",
    "test:browser": "aegir test -t browser -t webworker -f test/browser.js",
    "build": "aegir build",
    "lint": "aegir lint",
    "release": "aegir release",
    "release-minor": "aegir release --type minor",
    "release-major": "aegir release --type major",
-    "coverage": "aegir coverage"
+    "coverage": "aegir coverage",
+    "commit-lint": "commitlint --extends=@commitlint/config-conventional --from=remotes/origin/master --to=HEAD"


Let's not do this here, it'll come from aegir itself

Can I remove it when it's in aegir instead? Without this it's potluck as to whether Jenkins will quibble about the commit message or not. It'd be a much better experience to know about it ahead of time.

Fair enough, sounds reasonable.

victorb · 2018-05-08T16:49:36Z

package.json

@@ -35,37 +36,43 @@
  },
  "homepage": "https://github.com/ipfs/js-ipfs-mfs#readme",
  "devDependencies": {
+    "@commitlint/cli": "^6.2.0",
+    "@commitlint/config-conventional": "^6.1.3",


Same here, dependencies will come from aegir itself

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

daviddias · 2018-05-08T17:57:46Z

@achingbrain Does this PR implement the full MFS directory tree? What is the ETA to make it compatible with https://github.com/ipfs-shipyard/ipfs-blob-store/blob/master/src/mfs.js ?

achingbrain · 2018-05-08T18:09:20Z

@diasdavid each command is implemented, there are a few edge cases left but it's mostly there.

Not sure on the ETA - probably later this week? I was going to put this behind an experimental flag for js-ipfs tomorrow and run all the interface tests against it.

Thanks for that link - I notice that ipfs-blob-store expects ipfs.files.read to return a ReadableStream but the spec says it should return a Buffer so there will be those sort of inconsistencies to work out.

daviddias · 2018-05-09T06:33:44Z

@diasdavid each command is implemented, there are a few edge cases left but it's mostly there.

@achingbrain sweeeeet! That's awesome news. You might want to change the title of the PR. I saw code for a lot of things but the title suggested that things were far away from completion.

but the spec says it should return a Buffer so there will be those sort of inconsistencies to work out.

That is because we had ReadableStreams for everything when that was developed. Then we started shipping APIs that expose the trio Readable + Pull + Callback/Promise. All of that needs to be updated and propagated, you are correct!

achingbrain · 2018-05-09T07:24:31Z

You might want to change the title of the PR. I saw code for a lot of things but the title suggested that things were far away from completion.

Yeah - I didn't expect this to sit around for so long and was hoping to be able to submit a series of small PRs. I may just merge it and move on.

All of that needs to be updated and propagated

Do you mean there are supposed to be 3x methods for everything - read, readReadableStream and readPullStream, each one either taking a callback or returning a promise?

daviddias · 2018-05-09T07:37:42Z

Do you mean there are supposed to be 3x methods for everything - read, readReadableStream and readPullStream, each one either taking a callback or returning a promise?

Yes, so it follows the other APIs

achingbrain · 2018-05-09T08:53:07Z

Ok, I've opened ipfs-inactive/interface-js-ipfs-core#265 that adds those signatures to the spec for mfs files.read.

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

achingbrain self-assigned this May 2, 2018

daviddias reviewed May 3, 2018

View reviewed changes

achingbrain force-pushed the add-mfs-write-command branch from 2769c6f to 867d60f Compare May 8, 2018 15:41

achingbrain added 11 commits May 8, 2018 17:34

feat: basic mfs.write command

ccecb1b

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

chore: windows compat and downgrade node for leveldb

d06e73b

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

chore: decompose modules and reuse import logic for expanding files

6cf130f

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

feat: allow for truncating files

c515184

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

chore: use node lts version

db30286

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

feat: most of the cp command

5d189a6

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

feat: adds rm command

682c478

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

chore: adds tests for reading slices of mfs files

60baf97

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

feat: copy directories

cb0135c

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

feat: added mv command

1577094

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

chore: adds pre-commit hooks

23b4b34

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

achingbrain force-pushed the add-mfs-write-command branch from 867d60f to 23b4b34 Compare May 8, 2018 16:40

chore: simpler commit lint

3345b82

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

victorb reviewed May 8, 2018

View reviewed changes

chore: remove old ci but keep circle until jenkins coverage is working

ac213ee

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

achingbrain mentioned this pull request May 9, 2018

docs: adds stream-related mfs files.read method signatures ipfs-inactive/interface-js-ipfs-core#265

Merged

feat: implement streaming mfs.read methods

3e5620b

License: MIT Signed-off-by: achingbrain <alex@achingbrain.net>

achingbrain force-pushed the add-mfs-write-command branch from ec8cedf to 3e5620b Compare May 9, 2018 10:17

achingbrain merged commit affa0da into master May 9, 2018

daviddias deleted the add-mfs-write-command branch May 11, 2018 14:36

feat: Basic mfs.write command #1

feat: Basic mfs.write command #1

Uh oh!

Conversation

achingbrain commented May 2, 2018

Uh oh!

coveralls commented May 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 44

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achingbrain May 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daviddias commented May 8, 2018

Uh oh!

achingbrain commented May 8, 2018

Uh oh!

daviddias commented May 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achingbrain commented May 9, 2018

Uh oh!

daviddias commented May 9, 2018

Uh oh!

achingbrain commented May 9, 2018

Uh oh!

Uh oh!

coveralls commented May 2, 2018 •

edited

Loading

achingbrain May 3, 2018 •

edited

Loading

daviddias commented May 9, 2018 •

edited

Loading