GitHub PullRequestReviewCommentEvent at 2014-08-08 19:11:55
Repository: Dynamo
Actor: pboyer
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/comments/16010960","id":16010960,"diff_hunk":"@@ -0,0 +1,235 @@\n+﻿using System;","path":"src/DynamoCore/Models/DynamoModelEvents.cs","position":1,"original_position":1,"commit_id":"d28f5b28244febe13c0dc889a7719db87a629d91","original_commit_id":"54b3708090d09741c4f066d64f624c9c28d49a1a","user":{"login":"pboyer","id":916345,"avatar_url":"https://avatars.githubusercontent.com/u/916345?v=2","gravatar_id":"f50224de728451c482d3053cb7778c66","url":"https://api.github.com/users/pboyer","html_url":"https://github.com/pboyer","followers_url":"https://api.github.com/users/pboyer/followers","following_url":"https://api.github.com/users/pboyer/following{/other_user}","gists_url":"https://api.github.com/users/pboyer/gists{/gist_id}","starred_url":"https://api.github.com/users/pboyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pboyer/subscriptions","organizations_url":"https://api.github.com/users/pboyer/orgs","repos_url":"https://api.github.com/users/pboyer/repos","events_url":"https://api.github.com/users/pboyer/events{/privacy}","received_events_url":"https://api.github.com/users/pboyer/received_events","type":"User","site_admin":false},"body":"No problemo.  ","created_at":"2014-08-08T19:11:55Z","updated_at":"2014-08-08T19:11:55Z","html_url":"https://github.com/DynamoDS/Dynamo/pull/2055#discussion_r16010960","pull_request_url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055","_links":{"self":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/comments/16010960"},"html":{"href":"https://github.com/DynamoDS/Dynamo/pull/2055#discussion_r16010960"},"pull_request":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055"}}},"pull_request":{"url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055","id":19041700,"html_url":"https://github.com/DynamoDS/Dynamo/pull/2055","diff_url":"https://github.com/DynamoDS/Dynamo/pull/2055.diff","patch_url":"https://github.com/DynamoDS/Dynamo/pull/2055.patch","issue_url":"https://api.github.com/repos/DynamoDS/Dynamo/issues/2055","number":2055,"state":"open","title":"Remove dynSettings and DynamoController from all of Dynamo","user":{"login":"pboyer","id":916345,"avatar_url":"https://avatars.githubusercontent.com/u/916345?v=2","gravatar_id":"f50224de728451c482d3053cb7778c66","url":"https://api.github.com/users/pboyer","html_url":"https://github.com/pboyer","followers_url":"https://api.github.com/users/pboyer/followers","following_url":"https://api.github.com/users/pboyer/following{/other_user}","gists_url":"https://api.github.com/users/pboyer/gists{/gist_id}","starred_url":"https://api.github.com/users/pboyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pboyer/subscriptions","organizations_url":"https://api.github.com/users/pboyer/orgs","repos_url":"https://api.github.com/users/pboyer/repos","events_url":"https://api.github.com/users/pboyer/events{/privacy}","received_events_url":"https://api.github.com/users/pboyer/received_events","type":"User","site_admin":false},"body":"### The problem\r\n\r\nThe `dynSettings` class has for a long time represented a crutch in DynamoCore.  It provides static references to `DynamoModel` or `DynamoViewModel` (via `DynamoController`) from anywhere in the code.  It goes without saying that such a static data structure is bad idea, but here's a few:\r\n\r\n* It enables devs to easily break the separation between business, application, and presentation layers.\r\n* Makes it difficult to reason about what component knows about what - making debugging and testing more difficult.\r\n* It's impossible to initialize more than one Dynamo in the same app domain.\r\n* It's inconsistent with the rest of the MVVM based system.\r\n\r\nDynamoController is indirectly responsible for most of these things.  There is no strong reason for it to exist in DynamoCore and it only serves to confuse the MVVM based architecture.  It is also removed.  \r\n\r\n### The philosophy of the fix\r\n\r\n#### Use composition to make access explicit\r\n\r\nWe should force developers to explicitly provide access to the data structures they need in their component.  If the component needs access to another data structure, they should explicitly provide access in the constructor.  By making the requirements of their component explicit at creation time, developers can enforce the principal of least knowledge.\r\n\r\n```\r\nvar model = new Model();\r\nvar viewModel = new ViewModel(model);\r\nvar view = new View(viewModel);\r\n```\r\n\r\nPreviously, because of dynSettings, most of Dynamo did not follow this pattern.  Namely, classes in the Model layer often invoked methods on the ViewModel layer.  \r\n\r\n#### The right kind of communication\r\n\r\nObjects at any level should only be able to hold a reference to objects at the same level or a lower level.  For example, any type at the Model layer can have access to other Model types, but never a ViewModel type.  \r\n\r\n* Views communicate to ViewModels via command bindings.\r\n* ViewModels/Models notify about changes to Views via events (observables).  \r\n* ViewModels communicate to Models via method calls.\r\n* Models send messages to ViewModels/Views via events.  \r\n* Models communicate to other Models via method calls. \r\n\r\nThese contracts are now enforced in all of DynamoCore.\r\n\r\nWe can think about ViewModels as Models, just with the additional logic necessary to support commands and bindings, which are necessary for WPF support (but do not force the ViewModel to be WPF specific).\r\n\r\n### Other fixes\r\n \r\n* Moved `PackageLoader` to `DynamoLoader`\r\n* Added `NodeFactory` class to `WorkspaceModel`\r\n* Moved `DynamoRunner`, `Logger`, `CustomNodeManager`, `SearchModel` to `DynamoModel`\r\n* `AddConnector`, `AddNode`, `AddNote` methods moved to `WorkspaceModel`\r\n* Moved `VisualizationManager`, `WatchHandler` to DynamoViewModel\r\n* Separate `PackageManagerClient` into `PackageManagerClientViewModel` & `PackageManagerClient`\r\n* Separate `SearchViewModel` into `SearchViewModel` & `SearchModel`\r\n* Major cleanup of the `SearchElement` subtypes into Model types\r\n* Separate LibraryServices.cs into multiple files\r\n* Provide extensible `StartConfiguration` classes for `DynamoModel, RevitDynamoModel, DynamoViewModel`\r\n* Major cleanup to `DynamoRevit`\r\n\r\n### Reviewers \r\n\r\n@Benglin, @lukechurch, @ikeough, @Steell\r\n","created_at":"2014-07-29T17:32:24Z","updated_at":"2014-08-08T19:11:55Z","closed_at":null,"merged_at":null,"merge_commit_sha":"e99e181fd5e6dba002ea2ab3b9a2ea8a5f266db0","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055/commits","review_comments_url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055/comments","review_comment_url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/comments/{number}","comments_url":"https://api.github.com/repos/DynamoDS/Dynamo/issues/2055/comments","statuses_url":"https://api.github.com/repos/DynamoDS/Dynamo/statuses/d28f5b28244febe13c0dc889a7719db87a629d91","head":{"label":"pboyer:KillDynSettings","ref":"KillDynSettings","sha":"d28f5b28244febe13c0dc889a7719db87a629d91","user":{"login":"pboyer","id":916345,"avatar_url":"https://avatars.githubusercontent.com/u/916345?v=2","gravatar_id":"f50224de728451c482d3053cb7778c66","url":"https://api.github.com/users/pboyer","html_url":"https://github.com/pboyer","followers_url":"https://api.github.com/users/pboyer/followers","following_url":"https://api.github.com/users/pboyer/following{/other_user}","gists_url":"https://api.github.com/users/pboyer/gists{/gist_id}","starred_url":"https://api.github.com/users/pboyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pboyer/subscriptions","organizations_url":"https://api.github.com/users/pboyer/orgs","repos_url":"https://api.github.com/users/pboyer/repos","events_url":"https://api.github.com/users/pboyer/events{/privacy}","received_events_url":"https://api.github.com/users/pboyer/received_events","type":"User","site_admin":false},"repo":{"id":20731728,"name":"Dynamo","full_name":"pboyer/Dynamo","owner":{"login":"pboyer","id":916345,"avatar_url":"https://avatars.githubusercontent.com/u/916345?v=2","gravatar_id":"f50224de728451c482d3053cb7778c66","url":"https://api.github.com/users/pboyer","html_url":"https://github.com/pboyer","followers_url":"https://api.github.com/users/pboyer/followers","following_url":"https://api.github.com/users/pboyer/following{/other_user}","gists_url":"https://api.github.com/users/pboyer/gists{/gist_id}","starred_url":"https://api.github.com/users/pboyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pboyer/subscriptions","organizations_url":"https://api.github.com/users/pboyer/orgs","repos_url":"https://api.github.com/users/pboyer/repos","events_url":"https://api.github.com/users/pboyer/events{/privacy}","received_events_url":"https://api.github.com/users/pboyer/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/pboyer/Dynamo","description":"Visual Programming For Revit","fork":true,"url":"https://api.github.com/repos/pboyer/Dynamo","forks_url":"https://api.github.com/repos/pboyer/Dynamo/forks","keys_url":"https://api.github.com/repos/pboyer/Dynamo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pboyer/Dynamo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pboyer/Dynamo/teams","hooks_url":"https://api.github.com/repos/pboyer/Dynamo/hooks","issue_events_url":"https://api.github.com/repos/pboyer/Dynamo/issues/events{/number}","events_url":"https://api.github.com/repos/pboyer/Dynamo/events","assignees_url":"https://api.github.com/repos/pboyer/Dynamo/assignees{/user}","branches_url":"https://api.github.com/repos/pboyer/Dynamo/branches{/branch}","tags_url":"https://api.github.com/repos/pboyer/Dynamo/tags","blobs_url":"https://api.github.com/repos/pboyer/Dynamo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pboyer/Dynamo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pboyer/Dynamo/git/refs{/sha}","trees_url":"https://api.github.com/repos/pboyer/Dynamo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pboyer/Dynamo/statuses/{sha}","languages_url":"https://api.github.com/repos/pboyer/Dynamo/languages","stargazers_url":"https://api.github.com/repos/pboyer/Dynamo/stargazers","contributors_url":"https://api.github.com/repos/pboyer/Dynamo/contributors","subscribers_url":"https://api.github.com/repos/pboyer/Dynamo/subscribers","subscription_url":"https://api.github.com/repos/pboyer/Dynamo/subscription","commits_url":"https://api.github.com/repos/pboyer/Dynamo/commits{/sha}","git_commits_url":"https://api.github.com/repos/pboyer/Dynamo/git/commits{/sha}","comments_url":"https://api.github.com/repos/pboyer/Dynamo/comments{/number}","issue_comment_url":"https://api.github.com/repos/pboyer/Dynamo/issues/comments/{number}","contents_url":"https://api.github.com/repos/pboyer/Dynamo/contents/{+path}","compare_url":"https://api.github.com/repos/pboyer/Dynamo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pboyer/Dynamo/merges","archive_url":"https://api.github.com/repos/pboyer/Dynamo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pboyer/Dynamo/downloads","issues_url":"https://api.github.com/repos/pboyer/Dynamo/issues{/number}","pulls_url":"https://api.github.com/repos/pboyer/Dynamo/pulls{/number}","milestones_url":"https://api.github.com/repos/pboyer/Dynamo/milestones{/number}","notifications_url":"https://api.github.com/repos/pboyer/Dynamo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pboyer/Dynamo/labels{/name}","releases_url":"https://api.github.com/repos/pboyer/Dynamo/releases{/id}","created_at":"2014-06-11T16:06:09Z","updated_at":"2014-06-24T21:33:26Z","pushed_at":"2014-08-08T18:49:08Z","git_url":"git://github.com/pboyer/Dynamo.git","ssh_url":"git@github.com:pboyer/Dynamo.git","clone_url":"https://github.com/pboyer/Dynamo.git","svn_url":"https://github.com/pboyer/Dynamo","homepage":"http://dynamobim.org","size":542749,"stargazers_count":0,"watchers_count":0,"language":"C#","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"DynamoDS:master","ref":"master","sha":"8eec98b0d1887e70fe369b9b1122150dd422d153","user":{"login":"DynamoDS","id":6528209,"avatar_url":"https://avatars.githubusercontent.com/u/6528209?v=2","gravatar_id":null,"url":"https://api.github.com/users/DynamoDS","html_url":"https://github.com/DynamoDS","followers_url":"https://api.github.com/users/DynamoDS/followers","following_url":"https://api.github.com/users/DynamoDS/following{/other_user}","gists_url":"https://api.github.com/users/DynamoDS/gists{/gist_id}","starred_url":"https://api.github.com/users/DynamoDS/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DynamoDS/subscriptions","organizations_url":"https://api.github.com/users/DynamoDS/orgs","repos_url":"https://api.github.com/users/DynamoDS/repos","events_url":"https://api.github.com/users/DynamoDS/events{/privacy}","received_events_url":"https://api.github.com/users/DynamoDS/received_events","type":"Organization","site_admin":false},"repo":{"id":3395179,"name":"Dynamo","full_name":"DynamoDS/Dynamo","owner":{"login":"DynamoDS","id":6528209,"avatar_url":"https://avatars.githubusercontent.com/u/6528209?v=2","gravatar_id":null,"url":"https://api.github.com/users/DynamoDS","html_url":"https://github.com/DynamoDS","followers_url":"https://api.github.com/users/DynamoDS/followers","following_url":"https://api.github.com/users/DynamoDS/following{/other_user}","gists_url":"https://api.github.com/users/DynamoDS/gists{/gist_id}","starred_url":"https://api.github.com/users/DynamoDS/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DynamoDS/subscriptions","organizations_url":"https://api.github.com/users/DynamoDS/orgs","repos_url":"https://api.github.com/users/DynamoDS/repos","events_url":"https://api.github.com/users/DynamoDS/events{/privacy}","received_events_url":"https://api.github.com/users/DynamoDS/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/DynamoDS/Dynamo","description":"Visual Programming For Revit","fork":false,"url":"https://api.github.com/repos/DynamoDS/Dynamo","forks_url":"https://api.github.com/repos/DynamoDS/Dynamo/forks","keys_url":"https://api.github.com/repos/DynamoDS/Dynamo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/DynamoDS/Dynamo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/DynamoDS/Dynamo/teams","hooks_url":"https://api.github.com/repos/DynamoDS/Dynamo/hooks","issue_events_url":"https://api.github.com/repos/DynamoDS/Dynamo/issues/events{/number}","events_url":"https://api.github.com/repos/DynamoDS/Dynamo/events","assignees_url":"https://api.github.com/repos/DynamoDS/Dynamo/assignees{/user}","branches_url":"https://api.github.com/repos/DynamoDS/Dynamo/branches{/branch}","tags_url":"https://api.github.com/repos/DynamoDS/Dynamo/tags","blobs_url":"https://api.github.com/repos/DynamoDS/Dynamo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/DynamoDS/Dynamo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/DynamoDS/Dynamo/git/refs{/sha}","trees_url":"https://api.github.com/repos/DynamoDS/Dynamo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/DynamoDS/Dynamo/statuses/{sha}","languages_url":"https://api.github.com/repos/DynamoDS/Dynamo/languages","stargazers_url":"https://api.github.com/repos/DynamoDS/Dynamo/stargazers","contributors_url":"https://api.github.com/repos/DynamoDS/Dynamo/contributors","subscribers_url":"https://api.github.com/repos/DynamoDS/Dynamo/subscribers","subscription_url":"https://api.github.com/repos/DynamoDS/Dynamo/subscription","commits_url":"https://api.github.com/repos/DynamoDS/Dynamo/commits{/sha}","git_commits_url":"https://api.github.com/repos/DynamoDS/Dynamo/git/commits{/sha}","comments_url":"https://api.github.com/repos/DynamoDS/Dynamo/comments{/number}","issue_comment_url":"https://api.github.com/repos/DynamoDS/Dynamo/issues/comments/{number}","contents_url":"https://api.github.com/repos/DynamoDS/Dynamo/contents/{+path}","compare_url":"https://api.github.com/repos/DynamoDS/Dynamo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/DynamoDS/Dynamo/merges","archive_url":"https://api.github.com/repos/DynamoDS/Dynamo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/DynamoDS/Dynamo/downloads","issues_url":"https://api.github.com/repos/DynamoDS/Dynamo/issues{/number}","pulls_url":"https://api.github.com/repos/DynamoDS/Dynamo/pulls{/number}","milestones_url":"https://api.github.com/repos/DynamoDS/Dynamo/milestones{/number}","notifications_url":"https://api.github.com/repos/DynamoDS/Dynamo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/DynamoDS/Dynamo/labels{/name}","releases_url":"https://api.github.com/repos/DynamoDS/Dynamo/releases{/id}","created_at":"2012-02-09T07:35:33Z","updated_at":"2014-07-31T02:37:47Z","pushed_at":"2014-08-08T17:38:54Z","git_url":"git://github.com/DynamoDS/Dynamo.git","ssh_url":"git@github.com:DynamoDS/Dynamo.git","clone_url":"https://github.com/DynamoDS/Dynamo.git","svn_url":"https://github.com/DynamoDS/Dynamo","homepage":"http://dynamobim.org","size":699296,"stargazers_count":128,"watchers_count":128,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":100,"mirror_url":null,"open_issues_count":372,"forks":100,"open_issues":372,"watchers":128,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055"},"html":{"href":"https://github.com/DynamoDS/Dynamo/pull/2055"},"issue":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/issues/2055"},"comments":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/issues/2055/comments"},"review_comments":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055/comments"},"review_comment":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/pulls/2055/commits"},"statuses":{"href":"https://api.github.com/repos/DynamoDS/Dynamo/statuses/d28f5b28244febe13c0dc889a7719db87a629d91"}}}}