GitHub PullRequestReviewCommentEvent at 2014-12-12 11:10:04
Repository: certificate-transparency
Actor: pphaneuf
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/google/certificate-transparency/pulls/comments/21738491","id":21738491,"diff_hunk":"@@ -95,34 +99,31 @@ struct Participant {\n   }\n \n \n-  void ElectionMania(\n-      int num_rounds,\n-      const vector<std::unique_ptr<Participant>>* all_participants) {\n+  void ElectionMania(int num_rounds,\n+                     const vector<unique_ptr<Participant>>* all_participants) {\n     notification_.reset(new Notification);\n-    mania_thread_.reset(\n-        new std::thread([this, num_rounds, all_participants]() {\n-          for (int round(0); round < num_rounds; ++round) {\n-            VLOG(1) << id_ << \" starting round \" << round;\n-            ElectLikeABoss();\n-\n-            int num_masters(0);\n-            for (const std::unique_ptr<Participant>& participant :\n-                 *all_participants) {\n-              if (participant->election_->IsMaster()) {\n-                ++num_masters;\n-              }\n-            }\n-            // There /could/ be no masters if an update happened after we came\n-            // out of WaitToBecomeMaster, it's unlikely but possible.\n-            // There definitely shouldn't be > 1 master EVER, though.\n-            CHECK_LE(num_masters, 1) << \"From the PoV of \" << id_;\n-            StopElection();\n-            VLOG(1) << id_ << \" finished round \" << round;\n-            // election_.reset(new MasterElection(base_, &client_, dir_, id_));\n+    mania_thread_.reset(new thread([this, num_rounds, all_participants]() {\n+      for (int round(0); round < num_rounds; ++round) {\n+        VLOG(1) << id_ << \" starting round \" << round;\n+        ElectLikeABoss();\n+\n+        int num_masters(0);\n+        for (const auto& participant : *all_participants) {\n+          if (participant->election_->IsMaster()) {\n+            ++num_masters;\n           }\n-          VLOG(1) << id_ << \" Mania over!\";\n-          notification_->Notify();\n-        }));\n+        }\n+        // There /could/ be no masters if an update happened after we came\n+        // out of WaitToBecomeMaster, it's unlikely but possible.\n+        // There definitely shouldn't be > 1 master EVER, though.\n+        CHECK_LE(num_masters, 1) << \"From the PoV of \" << id_;\n+        StopElection();\n+        VLOG(1) << id_ << \" finished round \" << round;\n+        election_.reset(new MasterElection(base_, client_, dir_, id_));","path":"cpp/util/masterelection_test.cc","position":null,"original_position":114,"commit_id":"e72cf57a657e555f663313739507d76e9c46bef2","original_commit_id":"b5e373d500e8952eee6f46b9ac32713e026d791b","user":{"login":"pphaneuf","id":40400,"avatar_url":"https://avatars.githubusercontent.com/u/40400?v=3","gravatar_id":"","url":"https://api.github.com/users/pphaneuf","html_url":"https://github.com/pphaneuf","followers_url":"https://api.github.com/users/pphaneuf/followers","following_url":"https://api.github.com/users/pphaneuf/following{/other_user}","gists_url":"https://api.github.com/users/pphaneuf/gists{/gist_id}","starred_url":"https://api.github.com/users/pphaneuf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pphaneuf/subscriptions","organizations_url":"https://api.github.com/users/pphaneuf/orgs","repos_url":"https://api.github.com/users/pphaneuf/repos","events_url":"https://api.github.com/users/pphaneuf/events{/privacy}","received_events_url":"https://api.github.com/users/pphaneuf/received_events","type":"User","site_admin":false},"body":"Done.","created_at":"2014-12-12T11:10:04Z","updated_at":"2014-12-12T11:10:04Z","html_url":"https://github.com/google/certificate-transparency/pull/326#discussion_r21738491","pull_request_url":"https://api.github.com/repos/google/certificate-transparency/pulls/326","_links":{"self":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/comments/21738491"},"html":{"href":"https://github.com/google/certificate-transparency/pull/326#discussion_r21738491"},"pull_request":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/326"}}},"pull_request":{"url":"https://api.github.com/repos/google/certificate-transparency/pulls/326","id":25965662,"html_url":"https://github.com/google/certificate-transparency/pull/326","diff_url":"https://github.com/google/certificate-transparency/pull/326.diff","patch_url":"https://github.com/google/certificate-transparency/pull/326.patch","issue_url":"https://api.github.com/repos/google/certificate-transparency/issues/326","number":326,"state":"open","locked":false,"title":"Make it easy to run the masterelection_test with a real etcd.","user":{"login":"pphaneuf","id":40400,"avatar_url":"https://avatars.githubusercontent.com/u/40400?v=3","gravatar_id":"","url":"https://api.github.com/users/pphaneuf","html_url":"https://github.com/pphaneuf","followers_url":"https://api.github.com/users/pphaneuf/followers","following_url":"https://api.github.com/users/pphaneuf/following{/other_user}","gists_url":"https://api.github.com/users/pphaneuf/gists{/gist_id}","starred_url":"https://api.github.com/users/pphaneuf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pphaneuf/subscriptions","organizations_url":"https://api.github.com/users/pphaneuf/orgs","repos_url":"https://api.github.com/users/pphaneuf/repos","events_url":"https://api.github.com/users/pphaneuf/events{/privacy}","received_events_url":"https://api.github.com/users/pphaneuf/received_events","type":"User","site_admin":false},"body":"Also various smaller cleanups.","created_at":"2014-12-12T10:56:08Z","updated_at":"2014-12-12T11:10:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"73e62b39e0cf817493315d7bcbd77b5aca538c0a","assignee":{"login":"AlCutter","id":7648032,"avatar_url":"https://avatars.githubusercontent.com/u/7648032?v=3","gravatar_id":"","url":"https://api.github.com/users/AlCutter","html_url":"https://github.com/AlCutter","followers_url":"https://api.github.com/users/AlCutter/followers","following_url":"https://api.github.com/users/AlCutter/following{/other_user}","gists_url":"https://api.github.com/users/AlCutter/gists{/gist_id}","starred_url":"https://api.github.com/users/AlCutter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AlCutter/subscriptions","organizations_url":"https://api.github.com/users/AlCutter/orgs","repos_url":"https://api.github.com/users/AlCutter/repos","events_url":"https://api.github.com/users/AlCutter/events{/privacy}","received_events_url":"https://api.github.com/users/AlCutter/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/google/certificate-transparency/pulls/326/commits","review_comments_url":"https://api.github.com/repos/google/certificate-transparency/pulls/326/comments","review_comment_url":"https://api.github.com/repos/google/certificate-transparency/pulls/comments/{number}","comments_url":"https://api.github.com/repos/google/certificate-transparency/issues/326/comments","statuses_url":"https://api.github.com/repos/google/certificate-transparency/statuses/e72cf57a657e555f663313739507d76e9c46bef2","head":{"label":"pphaneuf:masterelection_test","ref":"masterelection_test","sha":"e72cf57a657e555f663313739507d76e9c46bef2","user":{"login":"pphaneuf","id":40400,"avatar_url":"https://avatars.githubusercontent.com/u/40400?v=3","gravatar_id":"","url":"https://api.github.com/users/pphaneuf","html_url":"https://github.com/pphaneuf","followers_url":"https://api.github.com/users/pphaneuf/followers","following_url":"https://api.github.com/users/pphaneuf/following{/other_user}","gists_url":"https://api.github.com/users/pphaneuf/gists{/gist_id}","starred_url":"https://api.github.com/users/pphaneuf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pphaneuf/subscriptions","organizations_url":"https://api.github.com/users/pphaneuf/orgs","repos_url":"https://api.github.com/users/pphaneuf/repos","events_url":"https://api.github.com/users/pphaneuf/events{/privacy}","received_events_url":"https://api.github.com/users/pphaneuf/received_events","type":"User","site_admin":false},"repo":{"id":19991870,"name":"certificate-transparency","full_name":"pphaneuf/certificate-transparency","owner":{"login":"pphaneuf","id":40400,"avatar_url":"https://avatars.githubusercontent.com/u/40400?v=3","gravatar_id":"","url":"https://api.github.com/users/pphaneuf","html_url":"https://github.com/pphaneuf","followers_url":"https://api.github.com/users/pphaneuf/followers","following_url":"https://api.github.com/users/pphaneuf/following{/other_user}","gists_url":"https://api.github.com/users/pphaneuf/gists{/gist_id}","starred_url":"https://api.github.com/users/pphaneuf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pphaneuf/subscriptions","organizations_url":"https://api.github.com/users/pphaneuf/orgs","repos_url":"https://api.github.com/users/pphaneuf/repos","events_url":"https://api.github.com/users/pphaneuf/events{/privacy}","received_events_url":"https://api.github.com/users/pphaneuf/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/pphaneuf/certificate-transparency","description":"Auditing for TLS certificates.","fork":true,"url":"https://api.github.com/repos/pphaneuf/certificate-transparency","forks_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/forks","keys_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/teams","hooks_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/hooks","issue_events_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/issues/events{/number}","events_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/events","assignees_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/assignees{/user}","branches_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/branches{/branch}","tags_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/tags","blobs_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/git/refs{/sha}","trees_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/statuses/{sha}","languages_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/languages","stargazers_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/stargazers","contributors_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/contributors","subscribers_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/subscribers","subscription_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/subscription","commits_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/commits{/sha}","git_commits_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/git/commits{/sha}","comments_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/comments{/number}","issue_comment_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/issues/comments/{number}","contents_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/contents/{+path}","compare_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/merges","archive_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/downloads","issues_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/issues{/number}","pulls_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/pulls{/number}","milestones_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/milestones{/number}","notifications_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/labels{/name}","releases_url":"https://api.github.com/repos/pphaneuf/certificate-transparency/releases{/id}","created_at":"2014-05-20T18:11:01Z","updated_at":"2014-12-11T18:16:43Z","pushed_at":"2014-12-12T11:09:54Z","git_url":"git://github.com/pphaneuf/certificate-transparency.git","ssh_url":"git@github.com:pphaneuf/certificate-transparency.git","clone_url":"https://github.com/pphaneuf/certificate-transparency.git","svn_url":"https://github.com/pphaneuf/certificate-transparency","homepage":"http://www.certificate-transparency.org/","size":20157,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"google:super_duper","ref":"super_duper","sha":"7520f282796a3add64939c47bb4ac8f0cde222d2","user":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=3","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"repo":{"id":19989605,"name":"certificate-transparency","full_name":"google/certificate-transparency","owner":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=3","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/google/certificate-transparency","description":"Auditing for TLS certificates.","fork":false,"url":"https://api.github.com/repos/google/certificate-transparency","forks_url":"https://api.github.com/repos/google/certificate-transparency/forks","keys_url":"https://api.github.com/repos/google/certificate-transparency/keys{/key_id}","collaborators_url":"https://api.github.com/repos/google/certificate-transparency/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/google/certificate-transparency/teams","hooks_url":"https://api.github.com/repos/google/certificate-transparency/hooks","issue_events_url":"https://api.github.com/repos/google/certificate-transparency/issues/events{/number}","events_url":"https://api.github.com/repos/google/certificate-transparency/events","assignees_url":"https://api.github.com/repos/google/certificate-transparency/assignees{/user}","branches_url":"https://api.github.com/repos/google/certificate-transparency/branches{/branch}","tags_url":"https://api.github.com/repos/google/certificate-transparency/tags","blobs_url":"https://api.github.com/repos/google/certificate-transparency/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/google/certificate-transparency/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/google/certificate-transparency/git/refs{/sha}","trees_url":"https://api.github.com/repos/google/certificate-transparency/git/trees{/sha}","statuses_url":"https://api.github.com/repos/google/certificate-transparency/statuses/{sha}","languages_url":"https://api.github.com/repos/google/certificate-transparency/languages","stargazers_url":"https://api.github.com/repos/google/certificate-transparency/stargazers","contributors_url":"https://api.github.com/repos/google/certificate-transparency/contributors","subscribers_url":"https://api.github.com/repos/google/certificate-transparency/subscribers","subscription_url":"https://api.github.com/repos/google/certificate-transparency/subscription","commits_url":"https://api.github.com/repos/google/certificate-transparency/commits{/sha}","git_commits_url":"https://api.github.com/repos/google/certificate-transparency/git/commits{/sha}","comments_url":"https://api.github.com/repos/google/certificate-transparency/comments{/number}","issue_comment_url":"https://api.github.com/repos/google/certificate-transparency/issues/comments/{number}","contents_url":"https://api.github.com/repos/google/certificate-transparency/contents/{+path}","compare_url":"https://api.github.com/repos/google/certificate-transparency/compare/{base}...{head}","merges_url":"https://api.github.com/repos/google/certificate-transparency/merges","archive_url":"https://api.github.com/repos/google/certificate-transparency/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/google/certificate-transparency/downloads","issues_url":"https://api.github.com/repos/google/certificate-transparency/issues{/number}","pulls_url":"https://api.github.com/repos/google/certificate-transparency/pulls{/number}","milestones_url":"https://api.github.com/repos/google/certificate-transparency/milestones{/number}","notifications_url":"https://api.github.com/repos/google/certificate-transparency/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/google/certificate-transparency/labels{/name}","releases_url":"https://api.github.com/repos/google/certificate-transparency/releases{/id}","created_at":"2014-05-20T17:03:47Z","updated_at":"2014-12-11T16:27:29Z","pushed_at":"2014-12-11T18:17:03Z","git_url":"git://github.com/google/certificate-transparency.git","ssh_url":"git@github.com:google/certificate-transparency.git","clone_url":"https://github.com/google/certificate-transparency.git","svn_url":"https://github.com/google/certificate-transparency","homepage":"http://www.certificate-transparency.org/","size":20118,"stargazers_count":73,"watchers_count":73,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":27,"mirror_url":null,"open_issues_count":27,"forks":27,"open_issues":27,"watchers":73,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/326"},"html":{"href":"https://github.com/google/certificate-transparency/pull/326"},"issue":{"href":"https://api.github.com/repos/google/certificate-transparency/issues/326"},"comments":{"href":"https://api.github.com/repos/google/certificate-transparency/issues/326/comments"},"review_comments":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/326/comments"},"review_comment":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/google/certificate-transparency/pulls/326/commits"},"statuses":{"href":"https://api.github.com/repos/google/certificate-transparency/statuses/e72cf57a657e555f663313739507d76e9c46bef2"}}}}