Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update all dependencies #190

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update all dependencies #190

wants to merge 2 commits into from

Conversation

G-Ray
Copy link

@G-Ray G-Ray commented Nov 21, 2018

I updated all the dependencies to the latest version.

torrent-discovery's api has changed a bit, do not hesitate to request changes if necessary.

@@ -779,7 +783,6 @@ var torrentStream = function (link, opts, cb) {
if (!port) return findPort(opts.port || DEFAULT_PORT, cb)
engine.port = port
swarm.listen(engine.port, cb)
discovery.updatePort(engine.port)
Copy link
Author

Choose a reason for hiding this comment

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

I removed this line, it should not be necessary right ?

Copy link
Owner

Choose a reason for hiding this comment

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

The discovery needs to announce the same port, is that done somewhere else now?

@G-Ray
Copy link
Author

G-Ray commented Nov 22, 2018

@mafintosh

@mafintosh
Copy link
Owner

Awesome stuff @G-Ray - See my one comment

peerId: bufferFrom(opts.id),
dht: (opts.dht !== undefined) ? opts.dht : true,
tracker: (opts.tracker !== undefined) ? opts.tracker : true,
port: engine.port || DEFAULT_PORT,
Copy link
Author

Choose a reason for hiding this comment

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

It should be done here right ?

@G-Ray
Copy link
Author

G-Ray commented Dec 22, 2019

I just re-updated this with the very latest deps. node >= 10 is now required due to bitfield v3.0.0. If you prefer to still support node < 10 we should keep using bitfield 2.x.

@@ -21,7 +21,7 @@ test('fixture can verify the torrent', function (t) {
t.plan(2)
fixture.on('ready', function () {
t.ok(true, 'seed should be ready')
t.deepEqual(fixture.bitfield.buffer.toString('hex'), 'c0', 'should verify all the pieces')
t.deepEqual(fixture.bitfield.buffer[0], 192, 'should verify all the pieces')
Copy link
Author

Choose a reason for hiding this comment

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

bitfield >= 3 should support node 8, but on travis we get

  ---expected    
  +++ actual      
  @@ -1,1 +1,1 @@ 
  -192            
  +0              
  test: test/auto-block.js fixture can verify the torrent
  stack: |
    Object.<anonymous> (test/auto-block.js:24:7)
    index.js:568:16

@pataquets
Copy link

Looks like this PR stalled. Although some dependencies are again outdated, if the effort invested is significant, I would merge this if possible and update again in another PR.
@G-Ray : Are you still interested/able to push it forward? Do you have the bandwidth and/or interest on it? No obligation, just wanting to make it obvious in case someone wants to pick it up. It would be a shame to lose all the time and effort put here.

@G-Ray
Copy link
Author

G-Ray commented Nov 11, 2020

Looks like this PR stalled. Although some dependencies are again outdated, if the effort invested is significant, I would merge this if possible and update again in another PR.
@G-Ray : Are you still interested/able to push it forward? Do you have the bandwidth and/or interest on it? No obligation, just wanting to make it obvious in case someone wants to pick it up. It would be a shame to lose all the time and effort put here.

Thank you @pataquets for your interest in this Pull Request ! 🤗
Unfortunately, I don't have enough bandwidth to update this PR these times. Feel free to fork my branch and reopen a PR, or I can give you write access to my fork to update this one.

@pataquets
Copy link

Fair enough. Thanks for the heads up, @G-Ray.
Unfortunately, I don't have the skills to pick this up, but it will be useful to know, no doubt. I would like to ask you to not delete your branch, in case anyone capable can pick it up.
I was interested since a project I use depended on it: https://github.com/KiraLT/torrent-stream-server .
However, since there are some security issues due to outdated modules, the above project switched to webtorrent, which is actively maintained.
Just FYI (or anyone else interested).
Thanks again for your work and time.
cc: @mafintosh

@mafintosh
Copy link
Owner

Using webtorrent makes sense 👍

@pataquets
Copy link

Thanks @mafintosh . In case you don't have interest or bandwidth for maintaining the project, may I suggest you to post a notice in the main readme?
Also, there's an interesting website I have discovered recently which might come handy, just in case: https://adoptoposs.org/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants