-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
feat(swarm): update swarm.peers to new api
Finished #600 Now only a test missing in core:
Which fails because ipfs.files.add never finishes for a file. Investigating |
Only 6 tests missing (really only 4, because 2 of them are the same thing, just api online and offline) bootstrap
patch addLink and rmLink
@dignifiedquire @haadcode @victorbjelkholm can I start getting a CR while I try to figure out these ones? So that we have it on time for tomorrow :) |
argv.key, | ||
{ enc: 'base58' }, | ||
cb) | ||
], (err, node) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please refrain from these cosmetic changes, I will change it back in the next commit any way and it makes it harder to see the relevant changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point! :)
} | ||
) | ||
} | ||
], (err) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a lot more complex and harder to read then before :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is also my attempt of trying to break each step by parts so that I can debug what is happening
return link.name !== linkRef.name | ||
} | ||
|
||
return !link.hash.equals(linkRef.hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about all the code above, why can we suddenly drop it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was duplicated logic, the addLink/rmLink operations on DAGNode are more complete now.
}, | ||
(cb) => { | ||
cb(null, linkedObj.multihash) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to use parallel
with callbacks anymore
0414b5e
to
5ff5cf9
Compare
5ff5cf9
to
3bb3ba8
Compare
e06f7dc
to
698f708
Compare
Now passing tests \o/ with the Caveat that Bootstrap add and rm fail, since it changed in js-ipfs-api and without having it standardized -- ipfs-inactive/interface-js-ipfs-core#97 --, it is hard to follow changes. |
918893d
to
7fad4d8
Compare
@@ -18,7 +18,7 @@ | |||
"npm": ">=3.0.0" | |||
}, | |||
"scripts": { | |||
"lint": "aegir-lint", | |||
"lint": "eslint -c node_modules/aegir/config/eslintrc.yml src test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linting is fine, it is just aegir-lint that is being mysterious. This is a known problem by @dignifiedquire, needs to be more investigated.
thanks @victorbjelkholm for the tip! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, but lets try to not actually merge this code, will see if I can figure this out on the aegir side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Q. Why did it pass with
eslint -c node_modules/aegir/config/eslintrc.yml src test
? - A. It did not lint
examples
- Q. Why does aegir fail silently?
- A. An eslint plugin being used in the examples folder: https://github.com/ipfs/js-ipfs/blob/master/examples/bundle-webpack/.eslintrc#L9 was missing, which resulted in an exit from
eslint
. Runningeslint
on the command line will print a custom error message for this, which does not happen when using gulp-eslint. - Q. Why does linting still fail?
- A. Because the examples were never properly fixed.
@@ -54,6 +54,7 @@ | |||
"buffer-loader": "0.0.1", | |||
"chai": "^3.5.0", | |||
"detect-node": "^2.0.3", | |||
"eslint-plugin-react": "^6.7.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, it was because of the added example? 💥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
b68c43a
to
2e59227
Compare
2e59227
to
20e3d2e
Compare
Tests suddenly started failing on Travis :( will try to investigate during my flight, locally and on circle is fine :S |
Found the issue, protocol-buffers was updated with a breaking change https://github.com/ipfs/js-ipfs-unixfs/blob/master/src/index.js#L54 type became an object: Will update once I land and get internet again :) @mafintosh was that breaking change on the last release intentional? |
CI is green again with new js-ipfs-unixfs released :) |
@diasdavid nope wasnt on purpose :( what broke in the latest release? |
found and fixed the regression. sry about that. |
thank you @mafintosh <3 |
facing some issues introduced with new swarms API, will focus on finishing this #600 first