{"action":"created","comment":{"url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/comments/176831047","pull_request_review_id":106608632,"id":176831047,"diff_hunk":"@@ -38,7 +38,8 @@\n     \"contributors\": \"*\",\n     \"eslint\": \"*\",\n     \"mocha\": \"*\",\n-    \"rewire\": \"*\"\n+    \"rewire\": \"*\",\n+    \"tmp\": \"^0.0.33\"","path":"package.json","position":23,"original_position":23,"commit_id":"4a44729782c87be55f46cc347905634699a3e98b","original_commit_id":"4a44729782c87be55f46cc347905634699a3e98b","user":{"login":"sbma44","id":31717,"avatar_url":"https://avatars2.githubusercontent.com/u/31717?v=4","gravatar_id":"","url":"https://api.github.com/users/sbma44","html_url":"https://github.com/sbma44","followers_url":"https://api.github.com/users/sbma44/followers","following_url":"https://api.github.com/users/sbma44/following{/other_user}","gists_url":"https://api.github.com/users/sbma44/gists{/gist_id}","starred_url":"https://api.github.com/users/sbma44/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sbma44/subscriptions","organizations_url":"https://api.github.com/users/sbma44/orgs","repos_url":"https://api.github.com/users/sbma44/repos","events_url":"https://api.github.com/users/sbma44/events{/privacy}","received_events_url":"https://api.github.com/users/sbma44/received_events","type":"User","site_admin":false},"body":"Yeah, this makes sense. Loose version requirements give me the willies in general but in this context I agree that there's no security issue and the potential for breakage in a dev environment is really just a matter of preference; happy to defer.","created_at":"2018-03-23T18:41:27Z","updated_at":"2018-03-23T18:41:27Z","html_url":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20#discussion_r176831047","pull_request_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/comments/176831047"},"html":{"href":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20#discussion_r176831047"},"pull_request":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20"}},"in_reply_to_id":176818623},"pull_request":{"url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20","id":177130099,"html_url":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20","diff_url":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20.diff","patch_url":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20.patch","issue_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/20","number":20,"state":"open","locked":false,"title":"[not ready] expanded tests & bugfixes","user":{"login":"sbma44","id":31717,"avatar_url":"https://avatars2.githubusercontent.com/u/31717?v=4","gravatar_id":"","url":"https://api.github.com/users/sbma44","html_url":"https://github.com/sbma44","followers_url":"https://api.github.com/users/sbma44/followers","following_url":"https://api.github.com/users/sbma44/following{/other_user}","gists_url":"https://api.github.com/users/sbma44/gists{/gist_id}","starred_url":"https://api.github.com/users/sbma44/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sbma44/subscriptions","organizations_url":"https://api.github.com/users/sbma44/orgs","repos_url":"https://api.github.com/users/sbma44/repos","events_url":"https://api.github.com/users/sbma44/events{/privacy}","received_events_url":"https://api.github.com/users/sbma44/received_events","type":"User","site_admin":false},"body":"refs #18 and #19 \r\n\r\n- adds offline tests for gzips, tarballs and purely-theoretical multi-mmdb tarballs (basically just for the sake of code coverage)\r\n- reinserts `use strict` to allow node 4 compatibility\r\n- fixes a bug in tarball handling that prevented callbacks from firing properly\r\n- adjusts behavior of tarball function to respect the `local` value in `config.js`, bringing it more in line with the project's earlier code\r\n- I have some lingering questions regarding the test for the top-level code path -- please see below\r\n- I bumped version a minor increment to 1.1.0. Happy to do otherwise if you have a strong preference; the changes to the `bin` script's interface felt like more than a patch, but I'm not sure it counts as a breaking change per se. The change to tarball behavior is substantially different but since 1.0.9 had a bug that rendered tarball handling unworkable, I don't think this should count as a breaking change either.\r\n\r\nHere's the most controversial part: I changed the existing behavior of `test/bin.js`. I'd like to talk about whether additional revisions are appropriate. Specifically: the previous test simply `require()`d the bin script. Failure could occur in the event of a syntax error. The act of requiring kicked off all the default network requests associated with invoking the script, but mocha's default test timeout (2000ms) killed most of these off before they completed, and their result wasn't checked anyway.\r\n\r\nIn order to unit test the tarball functionality I had to change the exported interface to the script (otherwise my new tests would kick off the full-dataset download process in the course of hooking into specific methods). In for a penny/in for a pound, I put a basic async wrapper around the script's primary codepath, tested for completion of the download loop and adjusted mocha's timeout.\r\n\r\nI am generally opposed writing tests that make network requests; but if we do make them, we should probably check their result. The project status quo makes those requests but aborts them and doesn't check the output. This strikes me as the worst of both worlds.\r\n\r\nSo! I'd like to suggest two options:\r\n- add some checks for completeness of downloaded files\r\n- stub out the `http` object further to handle the specific requests made in the first test, moving these tests fully offline\r\n\r\nI think the second option is preferable, but am eager to hear what you think @msimerson.","created_at":"2018-03-23T17:01:06Z","updated_at":"2018-03-23T18:41:27Z","closed_at":null,"merged_at":null,"merge_commit_sha":"44d01bb91ac49d5457bbc615039e0b0580190547","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20/commits","review_comments_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20/comments","review_comment_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/comments{/number}","comments_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/20/comments","statuses_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/statuses/4a44729782c87be55f46cc347905634699a3e98b","head":{"label":"mapbox:tarball-test","ref":"tarball-test","sha":"4a44729782c87be55f46cc347905634699a3e98b","user":{"login":"mapbox","id":600935,"avatar_url":"https://avatars1.githubusercontent.com/u/600935?v=4","gravatar_id":"","url":"https://api.github.com/users/mapbox","html_url":"https://github.com/mapbox","followers_url":"https://api.github.com/users/mapbox/followers","following_url":"https://api.github.com/users/mapbox/following{/other_user}","gists_url":"https://api.github.com/users/mapbox/gists{/gist_id}","starred_url":"https://api.github.com/users/mapbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mapbox/subscriptions","organizations_url":"https://api.github.com/users/mapbox/orgs","repos_url":"https://api.github.com/users/mapbox/repos","events_url":"https://api.github.com/users/mapbox/events{/privacy}","received_events_url":"https://api.github.com/users/mapbox/received_events","type":"Organization","site_admin":false},"repo":{"id":125899695,"name":"maxmind-geolite-mirror","full_name":"mapbox/maxmind-geolite-mirror","owner":{"login":"mapbox","id":600935,"avatar_url":"https://avatars1.githubusercontent.com/u/600935?v=4","gravatar_id":"","url":"https://api.github.com/users/mapbox","html_url":"https://github.com/mapbox","followers_url":"https://api.github.com/users/mapbox/followers","following_url":"https://api.github.com/users/mapbox/following{/other_user}","gists_url":"https://api.github.com/users/mapbox/gists{/gist_id}","starred_url":"https://api.github.com/users/mapbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mapbox/subscriptions","organizations_url":"https://api.github.com/users/mapbox/orgs","repos_url":"https://api.github.com/users/mapbox/repos","events_url":"https://api.github.com/users/mapbox/events{/privacy}","received_events_url":"https://api.github.com/users/mapbox/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mapbox/maxmind-geolite-mirror","description":"Mirror MaxMind GeoIP dbs from geolite.maxmind.com","fork":true,"url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror","forks_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/forks","keys_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/teams","hooks_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/hooks","issue_events_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/issues/events{/number}","events_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/events","assignees_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/assignees{/user}","branches_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/branches{/branch}","tags_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/tags","blobs_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/git/refs{/sha}","trees_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/statuses/{sha}","languages_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/languages","stargazers_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/stargazers","contributors_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/contributors","subscribers_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/subscribers","subscription_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/subscription","commits_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/commits{/sha}","git_commits_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/git/commits{/sha}","comments_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/comments{/number}","issue_comment_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/issues/comments{/number}","contents_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/contents/{+path}","compare_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/merges","archive_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/downloads","issues_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/issues{/number}","pulls_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/pulls{/number}","milestones_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/milestones{/number}","notifications_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/labels{/name}","releases_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/releases{/id}","deployments_url":"https://api.github.com/repos/mapbox/maxmind-geolite-mirror/deployments","created_at":"2018-03-19T18:02:21Z","updated_at":"2018-03-21T14:59:46Z","pushed_at":"2018-03-23T17:27:22Z","git_url":"git://github.com/mapbox/maxmind-geolite-mirror.git","ssh_url":"git@github.com:mapbox/maxmind-geolite-mirror.git","clone_url":"https://github.com/mapbox/maxmind-geolite-mirror.git","svn_url":"https://github.com/mapbox/maxmind-geolite-mirror","homepage":"https://www.npmjs.com/package/maxmind-geolite-mirror","size":75,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"msimerson:master","ref":"master","sha":"b337dcce8e394df40b7d1170ebeb83f811c28bd0","user":{"login":"msimerson","id":261635,"avatar_url":"https://avatars2.githubusercontent.com/u/261635?v=4","gravatar_id":"","url":"https://api.github.com/users/msimerson","html_url":"https://github.com/msimerson","followers_url":"https://api.github.com/users/msimerson/followers","following_url":"https://api.github.com/users/msimerson/following{/other_user}","gists_url":"https://api.github.com/users/msimerson/gists{/gist_id}","starred_url":"https://api.github.com/users/msimerson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/msimerson/subscriptions","organizations_url":"https://api.github.com/users/msimerson/orgs","repos_url":"https://api.github.com/users/msimerson/repos","events_url":"https://api.github.com/users/msimerson/events{/privacy}","received_events_url":"https://api.github.com/users/msimerson/received_events","type":"User","site_admin":false},"repo":{"id":28016247,"name":"maxmind-geolite-mirror","full_name":"msimerson/maxmind-geolite-mirror","owner":{"login":"msimerson","id":261635,"avatar_url":"https://avatars2.githubusercontent.com/u/261635?v=4","gravatar_id":"","url":"https://api.github.com/users/msimerson","html_url":"https://github.com/msimerson","followers_url":"https://api.github.com/users/msimerson/followers","following_url":"https://api.github.com/users/msimerson/following{/other_user}","gists_url":"https://api.github.com/users/msimerson/gists{/gist_id}","starred_url":"https://api.github.com/users/msimerson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/msimerson/subscriptions","organizations_url":"https://api.github.com/users/msimerson/orgs","repos_url":"https://api.github.com/users/msimerson/repos","events_url":"https://api.github.com/users/msimerson/events{/privacy}","received_events_url":"https://api.github.com/users/msimerson/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/msimerson/maxmind-geolite-mirror","description":"Mirror MaxMind GeoIP dbs from geolite.maxmind.com","fork":false,"url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror","forks_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/forks","keys_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/keys{/key_id}","collaborators_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/teams","hooks_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/hooks","issue_events_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/events{/number}","events_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/events","assignees_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/assignees{/user}","branches_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/branches{/branch}","tags_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/tags","blobs_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/git/refs{/sha}","trees_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/git/trees{/sha}","statuses_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/statuses/{sha}","languages_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/languages","stargazers_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/stargazers","contributors_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/contributors","subscribers_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/subscribers","subscription_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/subscription","commits_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/commits{/sha}","git_commits_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/git/commits{/sha}","comments_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/comments{/number}","issue_comment_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/comments{/number}","contents_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/contents/{+path}","compare_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/compare/{base}...{head}","merges_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/merges","archive_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/downloads","issues_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues{/number}","pulls_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls{/number}","milestones_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/milestones{/number}","notifications_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/labels{/name}","releases_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/releases{/id}","deployments_url":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/deployments","created_at":"2014-12-15T02:03:59Z","updated_at":"2018-03-20T16:32:44Z","pushed_at":"2018-03-23T17:27:23Z","git_url":"git://github.com/msimerson/maxmind-geolite-mirror.git","ssh_url":"git@github.com:msimerson/maxmind-geolite-mirror.git","clone_url":"https://github.com/msimerson/maxmind-geolite-mirror.git","svn_url":"https://github.com/msimerson/maxmind-geolite-mirror","homepage":"https://www.npmjs.com/package/maxmind-geolite-mirror","size":34,"stargazers_count":6,"watchers_count":6,"language":"JavaScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":3,"mirror_url":null,"archived":false,"open_issues_count":3,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":3,"open_issues":3,"watchers":6,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20"},"html":{"href":"https://github.com/msimerson/maxmind-geolite-mirror/pull/20"},"issue":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/20"},"comments":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/issues/20/comments"},"review_comments":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20/comments"},"review_comment":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/pulls/20/commits"},"statuses":{"href":"https://api.github.com/repos/msimerson/maxmind-geolite-mirror/statuses/4a44729782c87be55f46cc347905634699a3e98b"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"sbma44"}}