{"action":"created","issue":{"url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/687","repository_url":"https://api.github.com/repos/feross/webtorrent-desktop","labels_url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/687/labels{/name}","comments_url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/687/comments","events_url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/687/events","html_url":"https://github.com/feross/webtorrent-desktop/pull/687","id":162768352,"number":687,"title":"LocationHistory rework proposal","user":{"login":"Goldob","id":2529619,"avatar_url":"https://avatars.githubusercontent.com/u/2529619?v=3","gravatar_id":"","url":"https://api.github.com/users/Goldob","html_url":"https://github.com/Goldob","followers_url":"https://api.github.com/users/Goldob/followers","following_url":"https://api.github.com/users/Goldob/following{/other_user}","gists_url":"https://api.github.com/users/Goldob/gists{/gist_id}","starred_url":"https://api.github.com/users/Goldob/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Goldob/subscriptions","organizations_url":"https://api.github.com/users/Goldob/orgs","repos_url":"https://api.github.com/users/Goldob/repos","events_url":"https://api.github.com/users/Goldob/events{/privacy}","received_events_url":"https://api.github.com/users/Goldob/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2016-06-28T19:38:07Z","updated_at":"2016-07-28T23:02:58Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/feross/webtorrent-desktop/pulls/687","html_url":"https://github.com/feross/webtorrent-desktop/pull/687","diff_url":"https://github.com/feross/webtorrent-desktop/pull/687.diff","patch_url":"https://github.com/feross/webtorrent-desktop/pull/687.patch"},"body":"Relates with #653.\r\n\r\nThis is really just #656 after rebase. I decided to set up a new PR because Travis stopped responding to new commits and his output is crucial to illustrate the point (feel free to take a look at the unit tests).\r\n\r\n--\r\n\r\nWith the proposed changes, a procedure for transitioning from `old` to new `page` will work as follows:\r\n1. Call `new.onbeforeload` and wait for callback<sup>1</sup>\r\n2. Mark `new` as current page\r\n3. Call `old.onafterunload`<sup>2</sup>\r\n\r\n--\r\n\r\n<sup>1</sup> This was not always the case in the old version\r\n<sup>2</sup> I decided it is more suitable than `onbeforeunload`, because when we clean view-related variables, we want to do it **after** we stop rendering the page (or we risk errors otherwise). Also, this eliminates redundant complexity from having two `onbefores` - e.g. what if one has completed successfully, but the other returned an error? Do we just quietly abort the page change? But then the view whose function worked properly doesn't even know it's not being (un)loaded anymore. Maybe we should call the opposing function to notify it? What if it returns an error? You get the point."},"comment":{"url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/comments/236050687","html_url":"https://github.com/feross/webtorrent-desktop/pull/687#issuecomment-236050687","issue_url":"https://api.github.com/repos/feross/webtorrent-desktop/issues/687","id":236050687,"user":{"login":"feross","id":121766,"avatar_url":"https://avatars.githubusercontent.com/u/121766?v=3","gravatar_id":"","url":"https://api.github.com/users/feross","html_url":"https://github.com/feross","followers_url":"https://api.github.com/users/feross/followers","following_url":"https://api.github.com/users/feross/following{/other_user}","gists_url":"https://api.github.com/users/feross/gists{/gist_id}","starred_url":"https://api.github.com/users/feross/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/feross/subscriptions","organizations_url":"https://api.github.com/users/feross/orgs","repos_url":"https://api.github.com/users/feross/repos","events_url":"https://api.github.com/users/feross/events{/privacy}","received_events_url":"https://api.github.com/users/feross/received_events","type":"User","site_admin":false},"created_at":"2016-07-28T23:02:57Z","updated_at":"2016-07-28T23:02:57Z","body":"@Goldob Great changes! This is good work. Thanks for doing this. I prefer `tape` to `mocha` because it's a better citizen. It doesn't leak a bunch of globals and force you to write code that assumes they'll just be there.\r\n\r\n@dcposch I don't think we should have any unit tests in WebTorrent Desktop. This app should be mostly glue code. That can be tested best with a few sanity-check integration tests.\r\n\r\nIf there's a file that feels like it needs unit tests, then it should probably just be it's own npm package with it's own README, unit tests, etc. `LocationHistory` should definitely be it's own npm package. Probably, torrent-poster and a few others in `lib/` as well.\r\n\r\nI'm going to pull out `location-history` into it's own package today."}}
{"actor":{"display_login":"feross"}}