Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Spec MFS Actions #206

Merged
merged 11 commits into from
Jan 25, 2018
Merged

Spec MFS Actions #206

merged 11 commits into from
Jan 25, 2018

Conversation

hacdias
Copy link
Contributor

@hacdias hacdias commented Jan 22, 2018

Spec MFS functions:

  • ipfs.files.cp
  • ipfs.files.mkdir
  • ipfs.files.stat
  • ipfs.files.rm
  • ipfs.files.read
  • ipfs.files.write
  • ipfs.files.mv
  • ipfs.files.flush(path, [callback])

@ghost ghost assigned hacdias Jan 22, 2018
@ghost ghost added the in progress label Jan 22, 2018
@hacdias hacdias changed the title [WIP] Spec MFS Spec MFS Actions Jan 22, 2018
@hacdias
Copy link
Contributor Author

hacdias commented Jan 22, 2018

@diasdavid just finished adding the specs for the MFS specific functions. Everything I wrote can be seen from here.

Edit.: Probably I should write some tests too.

@hacdias hacdias requested review from daviddias and removed request for daviddias January 22, 2018 20:19
SPEC/FILES.md Outdated
@@ -539,6 +539,252 @@ pull(

A great source of [examples][] can be found in the tests for this API.

#### `cp`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MFS specific commands need a separator and an introduction to what MFS is.

Copy link
Contributor Author

@hacdias hacdias Jan 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay @diasdavid. It makes sense. Although, there is a command (ls) that was already spec'ed and it says that it is only for MFS but that it should be implemented globally. See.

@hacdias
Copy link
Contributor Author

hacdias commented Jan 23, 2018

@diasdavid just added a separator and a brief description. Although, I think that you could help me improve this description if you don't mind 😄

@ghost ghost assigned daviddias Jan 24, 2018
@daviddias
Copy link
Contributor

@hacdias updated wording. Seems you confused ipfs.ls and ipfs.files.ls. ipfs.files.ls is missing

@hacdias
Copy link
Contributor Author

hacdias commented Jan 24, 2018

Oh, it already said "files.ls" so I thought it would be the same. Thanks! I'll add the .Files.ls function today.

@hacdias
Copy link
Contributor Author

hacdias commented Jan 24, 2018

Done @diasdavid 😄

@daviddias
Copy link
Contributor

Sweet!

@hacdias can you add tests for these two? You will have to enable them to go-ipfs only. You can do it by checking:

https://github.com/ipfs/interface-ipfs-core/blob/master/src/key.js#L33
https://github.com/ipfs/interface-ipfs-core/blob/master/src/key.js#L178

@hacdias
Copy link
Contributor Author

hacdias commented Jan 25, 2018

@diasdavid tests done! 😄 They seem to work on ipfs-inactive/js-ipfs-http-client#675

src/files.js Outdated
describe('.mkdir', function () {
it('make directory on root', (done) => {
if (!withGo) {
console.log('Only supported by go-ipfs yet')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not supported in js-ipfs yet would be better since those logs will only be printed when running js-ipfs tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

src/files.js Outdated
@@ -980,5 +984,394 @@ module.exports = (common) => {
)
})
})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we break the MFS tests into another file? This test file is huge :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it should be added to index.js as a separate tests suit or should we just import them in files.js and there we would export them both?

@hacdias
Copy link
Contributor Author

hacdias commented Jan 25, 2018

@daviddias daviddias merged commit 7431098 into master Jan 25, 2018
@daviddias daviddias deleted the mfs branch January 25, 2018 18:22
@ghost ghost removed the in progress label Jan 25, 2018
@daviddias daviddias mentioned this pull request Jan 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants