{"action":"created","comment":{"url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/comments/103515343","pull_request_review_id":24304631,"id":103515343,"diff_hunk":"@@ -7,7 +7,7 @@ import styles from './header.css';\n \n const wordmark = 'https://assets.ubuntu.com/v1/d45097a4-snapcraft.io-logotype.svg';\n \n-class Header extends Component {\n+export class Header extends Component {","path":"src/common/components/header/index.js","position":null,"original_position":5,"commit_id":"ea0488487729f67f8acef15d1786e6b27270c517","original_commit_id":"4b564f4507a485725ab459799150e5c2c0823a0f","user":{"login":"nottrobin","id":519935,"avatar_url":"https://avatars1.githubusercontent.com/u/519935?v=3","gravatar_id":"","url":"https://api.github.com/users/nottrobin","html_url":"https://github.com/nottrobin","followers_url":"https://api.github.com/users/nottrobin/followers","following_url":"https://api.github.com/users/nottrobin/following{/other_user}","gists_url":"https://api.github.com/users/nottrobin/gists{/gist_id}","starred_url":"https://api.github.com/users/nottrobin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nottrobin/subscriptions","organizations_url":"https://api.github.com/users/nottrobin/orgs","repos_url":"https://api.github.com/users/nottrobin/repos","events_url":"https://api.github.com/users/nottrobin/events{/privacy}","received_events_url":"https://api.github.com/users/nottrobin/received_events","type":"User","site_admin":false},"body":"I've read this thread, the IRC conversation and [the article](https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.5vryhppki) @katiefenn mentioned.\r\n\r\nSince I'm still learning React, I don't have a specific opinion about this exact issue, but I do definitely believe that both encapsulation and a sensible separation of concerns are terribly important to try to prevent your codebase from growing into a spaghetti monster.\r\n\r\nI very much like the sound of separating \"Container\" and \"Presentational\" logic, and also of strictly exporting only one thing from each component, and of the idea that we should be doing our thinking up-front to save future developers from having to think.\r\n\r\nTrying to `export` more than one class sounds like it may be effectively trying to use global scope to make your life easier today. @katiefenn's suggestion on passing in properties instead, sounds similar to an example of [dependency injection](https://en.wikipedia.org/wiki/Dependency_injection), which is a widely accepted pattern for keeping neat, encapsulated units which are easier to test.\r\n\r\nIt is true that this can lead you to sometimes end up injecting an inordinately large number of dependencies. But this can be mitigated by sensibly breaking up your functions into smaller components, and then using [composition over inheritance](https://en.wikipedia.org/wiki/Composition_over_inheritance).\r\n\r\nAs I said, I don't understand enough to have a strong opinion about this specific piece of code, but since we are no longer under the gun, I do think it's worth taking the time to come up with patterns which ensure that concerns are separated, encapsulation is clean, and logical structures are kept as simple as possible.\r\n\r\nIf you do happen to come up with good rules of thumb, feel free to encode them into law by proposing a \"javascript.md\" or \"react.md\" into the [webteam practices coding folder](https://github.com/ubuntudesign/webteam-practices/tree/master/coding).","created_at":"2017-02-28T17:57:15Z","updated_at":"2017-02-28T17:57:15Z","html_url":"https://github.com/canonical-ols/build.snapcraft.io/pull/239#discussion_r103515343","pull_request_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239","_links":{"self":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/comments/103515343"},"html":{"href":"https://github.com/canonical-ols/build.snapcraft.io/pull/239#discussion_r103515343"},"pull_request":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239"}}},"pull_request":{"url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239","id":108302331,"html_url":"https://github.com/canonical-ols/build.snapcraft.io/pull/239","diff_url":"https://github.com/canonical-ols/build.snapcraft.io/pull/239.diff","patch_url":"https://github.com/canonical-ols/build.snapcraft.io/pull/239.patch","issue_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/239","number":239,"state":"open","locked":false,"title":"Handling empty GH user name","user":{"login":"bartaz","id":83575,"avatar_url":"https://avatars1.githubusercontent.com/u/83575?v=3","gravatar_id":"","url":"https://api.github.com/users/bartaz","html_url":"https://github.com/bartaz","followers_url":"https://api.github.com/users/bartaz/followers","following_url":"https://api.github.com/users/bartaz/following{/other_user}","gists_url":"https://api.github.com/users/bartaz/gists{/gist_id}","starred_url":"https://api.github.com/users/bartaz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bartaz/subscriptions","organizations_url":"https://api.github.com/users/bartaz/orgs","repos_url":"https://api.github.com/users/bartaz/repos","events_url":"https://api.github.com/users/bartaz/events{/privacy}","received_events_url":"https://api.github.com/users/bartaz/received_events","type":"User","site_admin":false},"body":"<img width=\"1061\" alt=\"screen shot 2017-02-28 at 11 30 11\" src=\"https://cloud.githubusercontent.com/assets/83575/23402097/7641718c-fdaa-11e6-89e0-c77bc8fecc38.png\">\r\n\r\nShowing GH login when full user name is not available.","created_at":"2017-02-28T10:39:05Z","updated_at":"2017-02-28T17:57:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a05defe1a5faada0653c0cc0ef18cdd155d861c2","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239/commits","review_comments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239/comments","review_comment_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/comments{/number}","comments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/239/comments","statuses_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/statuses/ea0488487729f67f8acef15d1786e6b27270c517","head":{"label":"canonical-ols:empty-gh-user-name","ref":"empty-gh-user-name","sha":"ea0488487729f67f8acef15d1786e6b27270c517","user":{"login":"canonical-ols","id":17836138,"avatar_url":"https://avatars2.githubusercontent.com/u/17836138?v=3","gravatar_id":"","url":"https://api.github.com/users/canonical-ols","html_url":"https://github.com/canonical-ols","followers_url":"https://api.github.com/users/canonical-ols/followers","following_url":"https://api.github.com/users/canonical-ols/following{/other_user}","gists_url":"https://api.github.com/users/canonical-ols/gists{/gist_id}","starred_url":"https://api.github.com/users/canonical-ols/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/canonical-ols/subscriptions","organizations_url":"https://api.github.com/users/canonical-ols/orgs","repos_url":"https://api.github.com/users/canonical-ols/repos","events_url":"https://api.github.com/users/canonical-ols/events{/privacy}","received_events_url":"https://api.github.com/users/canonical-ols/received_events","type":"Organization","site_admin":false},"repo":{"id":74463479,"name":"build.snapcraft.io","full_name":"canonical-ols/build.snapcraft.io","owner":{"login":"canonical-ols","id":17836138,"avatar_url":"https://avatars2.githubusercontent.com/u/17836138?v=3","gravatar_id":"","url":"https://api.github.com/users/canonical-ols","html_url":"https://github.com/canonical-ols","followers_url":"https://api.github.com/users/canonical-ols/followers","following_url":"https://api.github.com/users/canonical-ols/following{/other_user}","gists_url":"https://api.github.com/users/canonical-ols/gists{/gist_id}","starred_url":"https://api.github.com/users/canonical-ols/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/canonical-ols/subscriptions","organizations_url":"https://api.github.com/users/canonical-ols/orgs","repos_url":"https://api.github.com/users/canonical-ols/repos","events_url":"https://api.github.com/users/canonical-ols/events{/privacy}","received_events_url":"https://api.github.com/users/canonical-ols/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/canonical-ols/build.snapcraft.io","description":"The Snapcraft Build site.","fork":false,"url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io","forks_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/forks","keys_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/keys{/key_id}","collaborators_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/teams","hooks_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/hooks","issue_events_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/events{/number}","events_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/events","assignees_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/assignees{/user}","branches_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/branches{/branch}","tags_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/tags","blobs_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/refs{/sha}","trees_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/trees{/sha}","statuses_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/statuses/{sha}","languages_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/languages","stargazers_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/stargazers","contributors_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/contributors","subscribers_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/subscribers","subscription_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/subscription","commits_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/commits{/sha}","git_commits_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/commits{/sha}","comments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/comments{/number}","issue_comment_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/comments{/number}","contents_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/contents/{+path}","compare_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/compare/{base}...{head}","merges_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/merges","archive_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/downloads","issues_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues{/number}","pulls_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls{/number}","milestones_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/milestones{/number}","notifications_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/labels{/name}","releases_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/releases{/id}","deployments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/deployments","created_at":"2016-11-22T10:55:35Z","updated_at":"2017-02-27T08:20:28Z","pushed_at":"2017-02-28T17:46:36Z","git_url":"git://github.com/canonical-ols/build.snapcraft.io.git","ssh_url":"git@github.com:canonical-ols/build.snapcraft.io.git","clone_url":"https://github.com/canonical-ols/build.snapcraft.io.git","svn_url":"https://github.com/canonical-ols/build.snapcraft.io","homepage":"","size":686,"stargazers_count":4,"watchers_count":4,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":7,"mirror_url":null,"open_issues_count":27,"forks":7,"open_issues":27,"watchers":4,"default_branch":"master"}},"base":{"label":"canonical-ols:master","ref":"master","sha":"124b2362c3de3fe783a3dc6ea2af07d7d6977749","user":{"login":"canonical-ols","id":17836138,"avatar_url":"https://avatars2.githubusercontent.com/u/17836138?v=3","gravatar_id":"","url":"https://api.github.com/users/canonical-ols","html_url":"https://github.com/canonical-ols","followers_url":"https://api.github.com/users/canonical-ols/followers","following_url":"https://api.github.com/users/canonical-ols/following{/other_user}","gists_url":"https://api.github.com/users/canonical-ols/gists{/gist_id}","starred_url":"https://api.github.com/users/canonical-ols/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/canonical-ols/subscriptions","organizations_url":"https://api.github.com/users/canonical-ols/orgs","repos_url":"https://api.github.com/users/canonical-ols/repos","events_url":"https://api.github.com/users/canonical-ols/events{/privacy}","received_events_url":"https://api.github.com/users/canonical-ols/received_events","type":"Organization","site_admin":false},"repo":{"id":74463479,"name":"build.snapcraft.io","full_name":"canonical-ols/build.snapcraft.io","owner":{"login":"canonical-ols","id":17836138,"avatar_url":"https://avatars2.githubusercontent.com/u/17836138?v=3","gravatar_id":"","url":"https://api.github.com/users/canonical-ols","html_url":"https://github.com/canonical-ols","followers_url":"https://api.github.com/users/canonical-ols/followers","following_url":"https://api.github.com/users/canonical-ols/following{/other_user}","gists_url":"https://api.github.com/users/canonical-ols/gists{/gist_id}","starred_url":"https://api.github.com/users/canonical-ols/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/canonical-ols/subscriptions","organizations_url":"https://api.github.com/users/canonical-ols/orgs","repos_url":"https://api.github.com/users/canonical-ols/repos","events_url":"https://api.github.com/users/canonical-ols/events{/privacy}","received_events_url":"https://api.github.com/users/canonical-ols/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/canonical-ols/build.snapcraft.io","description":"The Snapcraft Build site.","fork":false,"url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io","forks_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/forks","keys_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/keys{/key_id}","collaborators_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/teams","hooks_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/hooks","issue_events_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/events{/number}","events_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/events","assignees_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/assignees{/user}","branches_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/branches{/branch}","tags_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/tags","blobs_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/refs{/sha}","trees_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/trees{/sha}","statuses_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/statuses/{sha}","languages_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/languages","stargazers_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/stargazers","contributors_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/contributors","subscribers_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/subscribers","subscription_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/subscription","commits_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/commits{/sha}","git_commits_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/git/commits{/sha}","comments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/comments{/number}","issue_comment_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/comments{/number}","contents_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/contents/{+path}","compare_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/compare/{base}...{head}","merges_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/merges","archive_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/downloads","issues_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues{/number}","pulls_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls{/number}","milestones_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/milestones{/number}","notifications_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/labels{/name}","releases_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/releases{/id}","deployments_url":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/deployments","created_at":"2016-11-22T10:55:35Z","updated_at":"2017-02-27T08:20:28Z","pushed_at":"2017-02-28T17:46:36Z","git_url":"git://github.com/canonical-ols/build.snapcraft.io.git","ssh_url":"git@github.com:canonical-ols/build.snapcraft.io.git","clone_url":"https://github.com/canonical-ols/build.snapcraft.io.git","svn_url":"https://github.com/canonical-ols/build.snapcraft.io","homepage":"","size":686,"stargazers_count":4,"watchers_count":4,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":7,"mirror_url":null,"open_issues_count":27,"forks":7,"open_issues":27,"watchers":4,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239"},"html":{"href":"https://github.com/canonical-ols/build.snapcraft.io/pull/239"},"issue":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/239"},"comments":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/issues/239/comments"},"review_comments":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239/comments"},"review_comment":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/pulls/239/commits"},"statuses":{"href":"https://api.github.com/repos/canonical-ols/build.snapcraft.io/statuses/ea0488487729f67f8acef15d1786e6b27270c517"}}}}
{"actor":{"display_login":"nottrobin"}}