{"action":"created","comment":{"url":"https://api.github.com/repos/holochain/holochain-rust/pulls/comments/211120598","pull_request_review_id":147444381,"id":211120598,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIxMTEyMDU5OA==","diff_hunk":"@@ -373,17 +362,21 @@ pub mod tests {\n         let dna = Dna::new();\n         let (sender, receiver) = channel();\n         instance.dispatch_with_observer(\n-            &ActionWrapper::new(&Action::InitApplication(dna.clone())),\n+            &ActionWrapper::new(Action::InitApplication(dna.clone())),\n             move |state: &State| match state.nucleus().dna() {\n                 Some(dna) => {\n-                    sender.send(dna).expect(\"test channel must be open\");\n-                    return true;\n+                    sender\n+                        .send(dna)\n+                        // the channel stays connected until the first message has been sent\n+                        // if this fails that means that it was called after having returned done=true\n+                        .expect(\"observer called after done\");","path":"core/src/instance.rs","position":290,"original_position":290,"commit_id":"ad750565082a8e847d4200ca6aa4cc474a2c51e8","original_commit_id":"ad750565082a8e847d4200ca6aa4cc474a2c51e8","user":{"login":"timotree3","id":6473438,"node_id":"MDQ6VXNlcjY0NzM0Mzg=","avatar_url":"https://avatars0.githubusercontent.com/u/6473438?v=4","gravatar_id":"","url":"https://api.github.com/users/timotree3","html_url":"https://github.com/timotree3","followers_url":"https://api.github.com/users/timotree3/followers","following_url":"https://api.github.com/users/timotree3/following{/other_user}","gists_url":"https://api.github.com/users/timotree3/gists{/gist_id}","starred_url":"https://api.github.com/users/timotree3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timotree3/subscriptions","organizations_url":"https://api.github.com/users/timotree3/orgs","repos_url":"https://api.github.com/users/timotree3/repos","events_url":"https://api.github.com/users/timotree3/events{/privacy}","received_events_url":"https://api.github.com/users/timotree3/received_events","type":"User","site_admin":false},"body":"see the comments","created_at":"2018-08-20T00:20:08Z","updated_at":"2018-08-20T00:55:59Z","html_url":"https://github.com/holochain/holochain-rust/pull/227#discussion_r211120598","pull_request_url":"https://api.github.com/repos/holochain/holochain-rust/pulls/227","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/comments/211120598"},"html":{"href":"https://github.com/holochain/holochain-rust/pull/227#discussion_r211120598"},"pull_request":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/227"}}},"pull_request":{"url":"https://api.github.com/repos/holochain/holochain-rust/pulls/227","id":209336985,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA5MzM2OTg1","html_url":"https://github.com/holochain/holochain-rust/pull/227","diff_url":"https://github.com/holochain/holochain-rust/pull/227.diff","patch_url":"https://github.com/holochain/holochain-rust/pull/227.patch","issue_url":"https://api.github.com/repos/holochain/holochain-rust/issues/227","number":227,"state":"open","locked":false,"title":"Transition to use more common rust idioms","user":{"login":"timotree3","id":6473438,"node_id":"MDQ6VXNlcjY0NzM0Mzg=","avatar_url":"https://avatars0.githubusercontent.com/u/6473438?v=4","gravatar_id":"","url":"https://api.github.com/users/timotree3","html_url":"https://github.com/timotree3","followers_url":"https://api.github.com/users/timotree3/followers","following_url":"https://api.github.com/users/timotree3/following{/other_user}","gists_url":"https://api.github.com/users/timotree3/gists{/gist_id}","starred_url":"https://api.github.com/users/timotree3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timotree3/subscriptions","organizations_url":"https://api.github.com/users/timotree3/orgs","repos_url":"https://api.github.com/users/timotree3/repos","events_url":"https://api.github.com/users/timotree3/events{/privacy}","received_events_url":"https://api.github.com/users/timotree3/received_events","type":"User","site_admin":false},"body":"This is a pretty big changelog. I'm open to feedback and I hope this doesn't come across as critical. I love this code.\r\n\r\n***Once I submit this PR, I'm going to write a review explaining each change that's not a pure formatting change.***\r\n\r\n**REVIEW IN PROGRESS. MAY BE DELAYED IF I LOSE INTERNET ON MY PLANE**\r\n\r\nHere are some \r\n## General themes:\r\n\r\n### Transition `unwrap()` -> `expect(\"how it should be\")`\r\n\r\nThis change is pretty pedantic.\r\n\r\nThe benefit of it is that if one of these `expect`s panics you'll get a descriptive error message that could be the difference between breaking your flow or not.\r\n\r\nI figure that if I'm going to do all the work to convert these unwraps then we may as well convert them.\r\n\r\n### Make functions own the arguments that they'd otherwise clone\r\n\r\nCloning takes resources and if the caller can choose whether to clone or just to move ownership to the function, that can be more efficient.\r\n\r\nFor example\r\n```rust\r\n#[derive(Clone, Debug)]\r\nstruct MyStruct(pub u64);\r\n\r\nfn clones_and_prints(arg: &MyStruct) {\r\n    println!(\"{:?}\", arg.clone());\r\n}\r\n\r\nfn takes_and_prints(arg: MyStruct) {\r\n    println!(\"{:?}\", arg)\r\n}\r\n\r\nfn worse_main() {\r\n    let to_print = MyStruct(4);\r\n    clones_and_prints(&to_print); // 1 clone\r\n}\r\n\r\nfn better_main() {\r\n    let to_print = MyStruct(4);\r\n    takes_and_prints(to_print); // no clones\r\n}\r\n```\r\n\r\n### Make functions return references to the objects that they'd otherwise clone\r\n\r\nThis is similar to above, gives the caller the choice of whether or not to clone. This helps in cases where cloning is unnecessary.\r\n\r\n### Formatting differences done by `cargo fmt`\r\n\r\nOOPS! I wish I hadn't run that command. :P Now the changes list is diluted.\r\n\r\n### Shifting certain patterns to their more common counterparts\r\n\r\nExamples:\r\n\r\n```rust\r\n// old way\r\nget_some_option().and_then(|v| Some(v+1))\r\n// new way\r\nget_some_option().map(|v| v+1)\r\n\r\n// old way\r\nget_some_result().or_else(|e| panic!(\"some message\"))\r\n// new way\r\nget_some_result().expect(\"some message\")\r\n\r\n// old way\r\nlet result = get_some_result();\r\nif result.is_err() {\r\n    return Err(\"some error\")\r\n}\r\nlet value = result.unwrap();\r\n// new way\r\nlet value = get_some_result().or(Err(\"some error\"))?;\r\n```","created_at":"2018-08-19T00:50:20Z","updated_at":"2018-08-20T00:55:57Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a4ea81b50f3575dbf060283b2bf4e560e0b978ef","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/holochain/holochain-rust/pulls/227/commits","review_comments_url":"https://api.github.com/repos/holochain/holochain-rust/pulls/227/comments","review_comment_url":"https://api.github.com/repos/holochain/holochain-rust/pulls/comments{/number}","comments_url":"https://api.github.com/repos/holochain/holochain-rust/issues/227/comments","statuses_url":"https://api.github.com/repos/holochain/holochain-rust/statuses/ad750565082a8e847d4200ca6aa4cc474a2c51e8","head":{"label":"timotree3:rust-idioms","ref":"rust-idioms","sha":"ad750565082a8e847d4200ca6aa4cc474a2c51e8","user":{"login":"timotree3","id":6473438,"node_id":"MDQ6VXNlcjY0NzM0Mzg=","avatar_url":"https://avatars0.githubusercontent.com/u/6473438?v=4","gravatar_id":"","url":"https://api.github.com/users/timotree3","html_url":"https://github.com/timotree3","followers_url":"https://api.github.com/users/timotree3/followers","following_url":"https://api.github.com/users/timotree3/following{/other_user}","gists_url":"https://api.github.com/users/timotree3/gists{/gist_id}","starred_url":"https://api.github.com/users/timotree3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timotree3/subscriptions","organizations_url":"https://api.github.com/users/timotree3/orgs","repos_url":"https://api.github.com/users/timotree3/repos","events_url":"https://api.github.com/users/timotree3/events{/privacy}","received_events_url":"https://api.github.com/users/timotree3/received_events","type":"User","site_admin":false},"repo":{"id":144340020,"node_id":"MDEwOlJlcG9zaXRvcnkxNDQzNDAwMjA=","name":"holochain-rust","full_name":"timotree3/holochain-rust","owner":{"login":"timotree3","id":6473438,"node_id":"MDQ6VXNlcjY0NzM0Mzg=","avatar_url":"https://avatars0.githubusercontent.com/u/6473438?v=4","gravatar_id":"","url":"https://api.github.com/users/timotree3","html_url":"https://github.com/timotree3","followers_url":"https://api.github.com/users/timotree3/followers","following_url":"https://api.github.com/users/timotree3/following{/other_user}","gists_url":"https://api.github.com/users/timotree3/gists{/gist_id}","starred_url":"https://api.github.com/users/timotree3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timotree3/subscriptions","organizations_url":"https://api.github.com/users/timotree3/orgs","repos_url":"https://api.github.com/users/timotree3/repos","events_url":"https://api.github.com/users/timotree3/events{/privacy}","received_events_url":"https://api.github.com/users/timotree3/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/timotree3/holochain-rust","description":"Holochain 2nd iteration in Rust with a Redux-like architecture and a WebAssembly Ribosome.","fork":true,"url":"https://api.github.com/repos/timotree3/holochain-rust","forks_url":"https://api.github.com/repos/timotree3/holochain-rust/forks","keys_url":"https://api.github.com/repos/timotree3/holochain-rust/keys{/key_id}","collaborators_url":"https://api.github.com/repos/timotree3/holochain-rust/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/timotree3/holochain-rust/teams","hooks_url":"https://api.github.com/repos/timotree3/holochain-rust/hooks","issue_events_url":"https://api.github.com/repos/timotree3/holochain-rust/issues/events{/number}","events_url":"https://api.github.com/repos/timotree3/holochain-rust/events","assignees_url":"https://api.github.com/repos/timotree3/holochain-rust/assignees{/user}","branches_url":"https://api.github.com/repos/timotree3/holochain-rust/branches{/branch}","tags_url":"https://api.github.com/repos/timotree3/holochain-rust/tags","blobs_url":"https://api.github.com/repos/timotree3/holochain-rust/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/timotree3/holochain-rust/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/timotree3/holochain-rust/git/refs{/sha}","trees_url":"https://api.github.com/repos/timotree3/holochain-rust/git/trees{/sha}","statuses_url":"https://api.github.com/repos/timotree3/holochain-rust/statuses/{sha}","languages_url":"https://api.github.com/repos/timotree3/holochain-rust/languages","stargazers_url":"https://api.github.com/repos/timotree3/holochain-rust/stargazers","contributors_url":"https://api.github.com/repos/timotree3/holochain-rust/contributors","subscribers_url":"https://api.github.com/repos/timotree3/holochain-rust/subscribers","subscription_url":"https://api.github.com/repos/timotree3/holochain-rust/subscription","commits_url":"https://api.github.com/repos/timotree3/holochain-rust/commits{/sha}","git_commits_url":"https://api.github.com/repos/timotree3/holochain-rust/git/commits{/sha}","comments_url":"https://api.github.com/repos/timotree3/holochain-rust/comments{/number}","issue_comment_url":"https://api.github.com/repos/timotree3/holochain-rust/issues/comments{/number}","contents_url":"https://api.github.com/repos/timotree3/holochain-rust/contents/{+path}","compare_url":"https://api.github.com/repos/timotree3/holochain-rust/compare/{base}...{head}","merges_url":"https://api.github.com/repos/timotree3/holochain-rust/merges","archive_url":"https://api.github.com/repos/timotree3/holochain-rust/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/timotree3/holochain-rust/downloads","issues_url":"https://api.github.com/repos/timotree3/holochain-rust/issues{/number}","pulls_url":"https://api.github.com/repos/timotree3/holochain-rust/pulls{/number}","milestones_url":"https://api.github.com/repos/timotree3/holochain-rust/milestones{/number}","notifications_url":"https://api.github.com/repos/timotree3/holochain-rust/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/timotree3/holochain-rust/labels{/name}","releases_url":"https://api.github.com/repos/timotree3/holochain-rust/releases{/id}","deployments_url":"https://api.github.com/repos/timotree3/holochain-rust/deployments","created_at":"2018-08-10T23:15:11Z","updated_at":"2018-08-10T23:15:13Z","pushed_at":"2018-08-19T00:37:07Z","git_url":"git://github.com/timotree3/holochain-rust.git","ssh_url":"git@github.com:timotree3/holochain-rust.git","clone_url":"https://github.com/timotree3/holochain-rust.git","svn_url":"https://github.com/timotree3/holochain-rust","homepage":null,"size":1894,"stargazers_count":0,"watchers_count":0,"language":"Rust","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":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0","node_id":"MDc6TGljZW5zZTk="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"develop"}},"base":{"label":"holochain:develop","ref":"develop","sha":"7f381f94c79b7476f5a1ba1fdccbc060c811b238","user":{"login":"holochain","id":29492745,"node_id":"MDEyOk9yZ2FuaXphdGlvbjI5NDkyNzQ1","avatar_url":"https://avatars2.githubusercontent.com/u/29492745?v=4","gravatar_id":"","url":"https://api.github.com/users/holochain","html_url":"https://github.com/holochain","followers_url":"https://api.github.com/users/holochain/followers","following_url":"https://api.github.com/users/holochain/following{/other_user}","gists_url":"https://api.github.com/users/holochain/gists{/gist_id}","starred_url":"https://api.github.com/users/holochain/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/holochain/subscriptions","organizations_url":"https://api.github.com/users/holochain/orgs","repos_url":"https://api.github.com/users/holochain/repos","events_url":"https://api.github.com/users/holochain/events{/privacy}","received_events_url":"https://api.github.com/users/holochain/received_events","type":"Organization","site_admin":false},"repo":{"id":133449383,"node_id":"MDEwOlJlcG9zaXRvcnkxMzM0NDkzODM=","name":"holochain-rust","full_name":"holochain/holochain-rust","owner":{"login":"holochain","id":29492745,"node_id":"MDEyOk9yZ2FuaXphdGlvbjI5NDkyNzQ1","avatar_url":"https://avatars2.githubusercontent.com/u/29492745?v=4","gravatar_id":"","url":"https://api.github.com/users/holochain","html_url":"https://github.com/holochain","followers_url":"https://api.github.com/users/holochain/followers","following_url":"https://api.github.com/users/holochain/following{/other_user}","gists_url":"https://api.github.com/users/holochain/gists{/gist_id}","starred_url":"https://api.github.com/users/holochain/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/holochain/subscriptions","organizations_url":"https://api.github.com/users/holochain/orgs","repos_url":"https://api.github.com/users/holochain/repos","events_url":"https://api.github.com/users/holochain/events{/privacy}","received_events_url":"https://api.github.com/users/holochain/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/holochain/holochain-rust","description":"Holochain 2nd iteration in Rust with a Redux-like architecture and a WebAssembly Ribosome.","fork":false,"url":"https://api.github.com/repos/holochain/holochain-rust","forks_url":"https://api.github.com/repos/holochain/holochain-rust/forks","keys_url":"https://api.github.com/repos/holochain/holochain-rust/keys{/key_id}","collaborators_url":"https://api.github.com/repos/holochain/holochain-rust/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/holochain/holochain-rust/teams","hooks_url":"https://api.github.com/repos/holochain/holochain-rust/hooks","issue_events_url":"https://api.github.com/repos/holochain/holochain-rust/issues/events{/number}","events_url":"https://api.github.com/repos/holochain/holochain-rust/events","assignees_url":"https://api.github.com/repos/holochain/holochain-rust/assignees{/user}","branches_url":"https://api.github.com/repos/holochain/holochain-rust/branches{/branch}","tags_url":"https://api.github.com/repos/holochain/holochain-rust/tags","blobs_url":"https://api.github.com/repos/holochain/holochain-rust/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/holochain/holochain-rust/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/holochain/holochain-rust/git/refs{/sha}","trees_url":"https://api.github.com/repos/holochain/holochain-rust/git/trees{/sha}","statuses_url":"https://api.github.com/repos/holochain/holochain-rust/statuses/{sha}","languages_url":"https://api.github.com/repos/holochain/holochain-rust/languages","stargazers_url":"https://api.github.com/repos/holochain/holochain-rust/stargazers","contributors_url":"https://api.github.com/repos/holochain/holochain-rust/contributors","subscribers_url":"https://api.github.com/repos/holochain/holochain-rust/subscribers","subscription_url":"https://api.github.com/repos/holochain/holochain-rust/subscription","commits_url":"https://api.github.com/repos/holochain/holochain-rust/commits{/sha}","git_commits_url":"https://api.github.com/repos/holochain/holochain-rust/git/commits{/sha}","comments_url":"https://api.github.com/repos/holochain/holochain-rust/comments{/number}","issue_comment_url":"https://api.github.com/repos/holochain/holochain-rust/issues/comments{/number}","contents_url":"https://api.github.com/repos/holochain/holochain-rust/contents/{+path}","compare_url":"https://api.github.com/repos/holochain/holochain-rust/compare/{base}...{head}","merges_url":"https://api.github.com/repos/holochain/holochain-rust/merges","archive_url":"https://api.github.com/repos/holochain/holochain-rust/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/holochain/holochain-rust/downloads","issues_url":"https://api.github.com/repos/holochain/holochain-rust/issues{/number}","pulls_url":"https://api.github.com/repos/holochain/holochain-rust/pulls{/number}","milestones_url":"https://api.github.com/repos/holochain/holochain-rust/milestones{/number}","notifications_url":"https://api.github.com/repos/holochain/holochain-rust/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/holochain/holochain-rust/labels{/name}","releases_url":"https://api.github.com/repos/holochain/holochain-rust/releases{/id}","deployments_url":"https://api.github.com/repos/holochain/holochain-rust/deployments","created_at":"2018-05-15T02:42:12Z","updated_at":"2018-08-19T20:31:08Z","pushed_at":"2018-08-19T15:54:09Z","git_url":"git://github.com/holochain/holochain-rust.git","ssh_url":"git@github.com:holochain/holochain-rust.git","clone_url":"https://github.com/holochain/holochain-rust.git","svn_url":"https://github.com/holochain/holochain-rust","homepage":null,"size":2049,"stargazers_count":82,"watchers_count":82,"language":"Rust","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":19,"mirror_url":null,"archived":false,"open_issues_count":116,"license":{"key":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0","node_id":"MDc6TGljZW5zZTk="},"forks":19,"open_issues":116,"watchers":82,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/227"},"html":{"href":"https://github.com/holochain/holochain-rust/pull/227"},"issue":{"href":"https://api.github.com/repos/holochain/holochain-rust/issues/227"},"comments":{"href":"https://api.github.com/repos/holochain/holochain-rust/issues/227/comments"},"review_comments":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/227/comments"},"review_comment":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/holochain/holochain-rust/pulls/227/commits"},"statuses":{"href":"https://api.github.com/repos/holochain/holochain-rust/statuses/ad750565082a8e847d4200ca6aa4cc474a2c51e8"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"timotree3"}}