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

Fix ratio limit in documentation #1338

Merged
merged 1 commit into from Mar 25, 2018
Merged

Conversation

@qgustavor
Copy link
Contributor

qgustavor commented Mar 25, 2018

The value of ratio isn't limited to 0-1, which can be checked here and here. It probably was a mistake caused by copy-pasting the documentation of "progress", which is limited to 0-1. The commit where this extra info was added is this one.

As this value is the result of something like uploaded / max(1, downloaded) it never get's to Infinity, so it's limited by 0 and uploaded. Without the || 1 part it would be limited between 0 and infinity.

The value of ratio isn't limited to 0-1, which can be checked in the two first links below. It probably was a mistake caused by copy-pasting the documentation of "progress", which is limited to 0-1. The commit where this extra info was added is the one in the third link.

- https://github.com/webtorrent/webtorrent/blob/b53d224cdea46173ea97dc9eea7eaf5a6e7a55b9/lib/torrent.js#L177-L179
- https://github.com/webtorrent/webtorrent/blob/b53d224cdea46173ea97dc9eea7eaf5a6e7a55b9/index.js#L195-L205
- 6f2e85c

As this value is the result of something like `uploaded / max(1, downloaded)` it never get's to `Infinity`, so it's limited by 0 and `uploaded`. Without the ` || 1` part it would be limited between 0 and infinity.
@DiegoRBaquero DiegoRBaquero merged commit 1dcc589 into webtorrent:master Mar 25, 2018
3 checks passed
3 checks passed
Node Security No known vulnerabilities found
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lock lock bot locked as resolved and limited conversation to collaborators Jun 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.