search

ipfs-inactive / js-ipfs-unixfs-engine

[ARCHIVED] JavaScript implementation of the layout and chunking mechanisms used by IPFS
MIT License
22 stars 20 forks source link

Modularization not necessary #237

Closed daviddias closed 4 years ago

daviddias commented 5 years ago

This https://github.com/ipfs/js-ipfs-unixfs-engine/blob/master/src/index.js#L3-L4

exports.importer = exports.Importer = require('ipfs-unixfs-importer') exports.exporter = exports.Exporter = require('ipfs-unixfs-exporter')

Specially when testing the exporter requires the importer.

Can this be reverted please?

achingbrain commented 5 years ago

Do you mean fold the exporter & importer back into this repo?

I'd like to see the ipfs-unixfs-exporter renamed as ipfs-exporter as it already handles exporting raw and dag-cbor nodes which are not strictly part of unixfs but can be encountered in DAGs so need to be handled.

At that point the ipfs-exporter using the unixfs-importer in it's tests to test unixfs stuff doesn't seem so bad, at least to me.

vmx commented 5 years ago

I'm also in favour of the separation. When I did some js-ipld/js-ipld-dag-pb refactoring having separate smaller unites really helped.

daviddias commented 4 years ago

Having this module as a shell just to import two other modules doesn't make sense, it would be better off just moving these two lines into js-ipfs itself https://github.com/ipfs/js-ipfs-unixfs-engine/blob/master/src/index.js#L3-L4

I'm happy that you did the right thing of moving the commit history to importer and exporter, I would be sad if all the contributions from the many individuals that contributed over time got lost.

daviddias commented 4 years ago

nvm. Got my answer at https://github.com/ipfs/js-ipfs/pull/2595#discussion_r345013516