-
Notifications
You must be signed in to change notification settings - Fork 1.2k
runtime specific things - bring libp2p bundles here #895
Conversation
src/core/components/libp2p.js
Outdated
@@ -1,6 +1,6 @@ | |||
'use strict' | |||
|
|||
const Node = require('libp2p-ipfs-nodejs') | |||
const Node = require('../runtime/libp2p-nodejs') |
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.
Perhaps add a comment before explaining that this will be switched to file x in the browser?
} | ||
} | ||
|
||
class Node extends libp2p { |
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 file has a lot in common with it's browser version. Perhaps we could reduce this to a declarative config and not need to declare a class here?
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.
True, at the same time it serves the purpose of being an awesome template for others to c&p
@@ -32,6 +32,7 @@ module.exports = (ctl) => { | |||
ctl.bootstrap.add({ default: true }, (err, res) => { | |||
expect(err).to.not.exist() | |||
peers = res.Peers | |||
console.log(res) |
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.
letfover console.log?
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.
LGTM overall, but I would still to try to reduce node-browser similar code (see comments).
* feat: move runtime dependent configs (repo and config) to runtime folder * feat: bring libp2p bundles to this repo
Following: libp2p/js-libp2p#92
Moving bundles and tests from libp2p-ipfs-* to js-libp2p