Skip to content

/ webtorrent

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

Move tracker options into `opts.tracker` (#649) #791

Closed
wants to merge 2 commits into from
Closed

Move tracker options into `opts.tracker` (#649) #791

wants to merge 2 commits into from

Conversation

@nkittsteiner
Copy link

nkittsteiner commented May 7, 2016

Moving wtrc and rtcConfig opts to opts.tracker. Hope it helps :)
nkittsteiner added 2 commits May 6, 2016
@nkittsteiner
WIP running tests for changes in opts
c5429d6
@nkittsteiner
Changing documentation, deleting wrtc and rtcConfig opts
Loading status checks…
5375ea4
index.js
if (typeof self.tracker === 'object') {
self.tracker.rtcConfig = opts.rtcConfig
self.tracker.wrtc = opts.wrtc || global.WRTC // to support `webtorrent-hybrid` package
}

This comment has been minimized.

Sign in to view
Copy link
@feross

feross May 8, 2016

Member

These options should be passed into the tracker constructor, not set directly on the tracker object like this.

This comment has been minimized.

Sign in to view
Copy link
@feross

feross May 8, 2016

Member

Nevermind.
@nkittsteiner

This comment has been minimized.

Sign in to view
Copy link
Author

nkittsteiner commented May 8, 2016

Hi Feross, do you mean with nevermind leave it like this? Because I can fix that
@feross

This comment has been minimized.

Sign in to view
Copy link
Member

feross commented May 8, 2016

@nkittsteiner Thanks a lot for this PR. It illuminated a few issues that I hadn't though of, like how to handle opts.rtcConfig and opts.wrtc, as well as how to handle duplicate getAnnounceOpts functions passed into opts.tracker and also into the opts for client.add.

I'll send a PR that builds on yours.
feross added a commit that referenced this pull request May 8, 2016
@feross
Move tracker options into `opts.tracker`
Loading status checks…
47d181f 
Closes #649

Based on a PR by @nkittsteiner. #791
@feross feross mentioned this pull request May 8, 2016
Merged
@feross

This comment has been minimized.

Sign in to view
Copy link
Member

feross commented May 8, 2016

See: #793
@feross feross closed this May 8, 2016
@lock

This comment has been minimized.

Sign in to view
Copy link

lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.
@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

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