Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upremove `self = this` in torrent.js #1487
Conversation
This comment has been minimized.
This comment has been minimized.
request-info
bot
commented
Aug 26, 2018
|
|
KayleePop
changed the title
| } | ||
| } | ||
|
|
||
| _getMetadataFromServer () { | ||
| // to allow function hoisting |
This comment has been minimized.
This comment has been minimized.
KayleePop
Aug 26, 2018
Author
Contributor
Hoisting was used here for readability, and hoisting is impossible with arrow functions
| } | ||
| } | ||
|
|
||
| /** | ||
| * Attempts to update a peer's requests | ||
| */ | ||
| _updateWire (wire) { | ||
| // to allow function hoisting |
This comment has been minimized.
This comment has been minimized.
|
|
||
| // TODO: what is this for? | ||
| if (!self.ready) return self.once('ready', () => { onChunk(err, chunk) }) | ||
| if (!this.ready) await new Promise(resolve => this.once('ready', resolve)) |
This comment has been minimized.
This comment has been minimized.
KayleePop
Aug 26, 2018
Author
Contributor
I used await and an async function here to allow this callback to be an anonymous arrow function. I think that's OK because async functions are supported in every browser that supports webRTC, and in node 8 which is LTS. (the tests wouldn't pass otherwise as they're set to the latest LTS in travis)
| const reconnectTimeout = setTimeout(function reconnectTimeout () { | ||
| const newPeer = self._addPeer(peer.addr) | ||
| const reconnectTimeout = setTimeout(() => { | ||
| const newPeer = this._addPeer(peer.addr) |
This comment has been minimized.
This comment has been minimized.
KayleePop
Aug 26, 2018
Author
Contributor
Did this timeout callback need to be named? I don't think so.
This comment has been minimized.
This comment has been minimized.
|
We are not implementing async/await just yet FYI. Everything for now should
work in node 6
…On Sun, Aug 26, 2018, 1:59 PM Kaylee ***@***.***> wrote:
*@KayleePop* commented on this pull request.
------------------------------
In lib/torrent.js
<#1487 (comment)>
:
>
// TODO: what is this for?
- if (!self.ready) return self.once('ready', () => { onChunk(err, chunk) })
+ if (!this.ready) await new Promise(resolve => this.once('ready', resolve))
I used await and an async function here to allow this callback to be an
anonymous arrow function. I think that's OK because async functions are
supported in every browser that supports webRTC, and in node 8 which is
LTS. (the tests wouldn't pass otherwise as they're set to the latest LTS in
travis)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1487 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB6fN0hNjZXQRlXLaFkdk7x3AyHdW1RWks5uUvAlgaJpZM4WM4Cj>
.
|
| self.discovery.on('peer', onPeer) | ||
| self.discovery.on('trackerAnnounce', onTrackerAnnounce) | ||
| self.discovery.on('dhtAnnounce', onDHTAnnounce) | ||
| self.discovery.on('warning', onWarning) |
This comment has been minimized.
This comment has been minimized.
KayleePop
Aug 26, 2018
Author
Contributor
Changed these to in-line arrow functions instead of being hoisted. IMO it's actually more readable that way, but it's a change in style.
This comment has been minimized.
This comment has been minimized.
|
reverted changes to _request() It just uses self still. |
KayleePop
changed the title
|
LGTM |
KayleePop commentedAug 26, 2018
No description provided.