version

[daviddias]

• cli
• http-api
• core
• tests cli
• tests http-api
• tests core

[dignifiedquire]

Hmm, this looks like asking for trouble when the export is only available after start was called, also what will happen to module caching plus calling start twice?

[daviddias]

the .ipfs is to be used by the http-api resources, which are only loaded after start is called (from routes), so there is no conflict. Did I get your question right?

[dignifiedquire]

Kind of, but why not just pass these resources through explicitly, this could easily break, or be miss used by someone not understanding these exports

[daviddias]

going to make it all es6'ish before merging it :)

[dignifiedquire]

This could be much cleaner:

server.route(require('./routes/version.js')

// version.js
var resources = require('./../resources')

module.exports = {
  method: 'GET',
  path: '/api/v0/version',
  handler: resources.version.get
}

[daviddias]

why is that "much" cleaner? If there was more than one route on that file I would have to go and

server.route(require('./routes/version.js').routeA)
server.route(require('./routes/version.js').routeB)
// etc

Meaning that I have to alter the route file and index.js, the way I have, I only have to open and modify one file. Also, having the server object available means I always have server methods at my hand.

[dignifiedquire]

It's not about that, could also change it to

require('./routes/version.js').forEach(server.route.bind(server)

// version.js
module.exports = [
  // my routes
]

The main issue I have your current implementation is that it introduces a circular dependency from this file to the routes files.

If you want to have the server object be available make it like this:

require('./routes/version.js')(server)

// version.js
server.route({...})

that way the circular dependency is removed

[daviddias]

we could DI the server obj (and the IPFS core object), but I kind of like making available by knowing where to pick it from other then move it around. I've been doing this for a while, if it becomes complicated over time, I will reconsider your suggestion :)

[dignifiedquire]

Can be const

[daviddias]

Thank you for the CR @dignifiedquire :)

[dignifiedquire]

You can drop the parens around callback

[dignifiedquire]

If you do this check anyway, why not go class IPFS instead?

[daviddias]

I was not sure how classes worked underneath, was afraid that it would cause the same problem of method allocation during object initialisation and take a lot of unnecessary memory, but after checking, it seems there is no reason why to be concerned since it moves all the methods to .prototype of class.

[dignifiedquire]

Thank you for the CR @dignifiedquire :)

I see code, I review