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

Feature/continue seeding after closing player #607

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

Conversation

chtrinh
Copy link

@chtrinh chtrinh commented Feb 14, 2017

Uses a forked process with WebTorrent client and saved torrents in cache tmp directory to accomplish this.

'autoSeed' - starts up a forked process with a WebTorrent client which
enables this feature. Torrent files are create and saved in tmp folder
to be used later for seeding. Randomly selects files for join swarm.
'seedLimit' - starting limit for number of torrents connected.
Copy link
Member

@xaiki xaiki left a comment

Choose a reason for hiding this comment

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

heads up this is a very important patch for us so it might be a bit of time in review =)

it looks like it's missing the 'named seeder' thing we talked about.

also, we probably want to hook this up into the torrent updater code too

still has a lot of whitespace changes.

<!-- Global variables -->
<script src="global.js"></script>
<!-- Global variables -->
<script src="global.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

still has whitespace issues

Copy link
Author

Choose a reason for hiding this comment

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

are you guys using tabs or spaces here? jshint doesn't seem to lint for me..

@@ -84,32 +85,32 @@
<script src="lib/models/lang.js"></script>
<script src="lib/models/content_item.js"></script>
<script src="lib/models/movie.js"></script>
<script src="lib/models/show.js"></script>
<script src="lib/models/show.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

still has whitespace issues

<script src="lib/models/generic_collection.js"></script>
<script src="lib/models/movie_collection.js"></script>
<script src="lib/models/stream_info.js"></script>
<script src="lib/models/show_collection.js"></script>
<script src="lib/models/anime_collection.js"></script>
<script src="lib/models/anime_collection.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

still has whitespace issues

<script src="lib/models/indie_collection.js"></script>
<script src="lib/models/favorite_collection.js"></script>
<script src="lib/models/watchlist_collection.js"></script>
<script src="lib/models/watchlist_collection.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

still has whitespace issues


// Focus the window when the app opens
win.focus();

Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be removed

Copy link
Author

Choose a reason for hiding this comment

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

opps will add back.

getTorrentFiles: function() {
if (this.torrentFiles === null) {
var regexp = /\.torrent$/i;
var files = fs.readdirSync(this.torrentDir);
Copy link
Member

Choose a reason for hiding this comment

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

instead of calling fs here, i think i'd rather maintain a registery of things that have been 'added/removed' to this module.

seedLimit: Settings.seedLimit
});

this.worker = child.fork(taskFile, [args], {silent: true, execPath:'node'});
Copy link
Member

Choose a reason for hiding this comment

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

'node' will fail on machines that don't have nodejs installed (we bundle)

@chtrinh
Copy link
Author

chtrinh commented Feb 14, 2017

@xaiki: yup no worries about the time it'll take. I got other PRs bug/suggestions to make :)

@xaiki
Copy link
Member

xaiki commented Apr 26, 2017

hey any news on this ?

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.

2 participants