GitHub PullRequestReviewCommentEvent at 2014-08-20 21:25:26
Repository: chromeos-filesystems
Actor: lavelle
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/comments/16506706","id":16506706,"diff_hunk":"@@ -18,30 +17,166 @@ var events = {\n   onReadDirectoryRequested: require('./events/onReadDirectoryRequested')\n };\n \n-window.onload = function() {\n-  var key = credentials.key;\n-  var secret = credentials.secret;\n-  var region = 'us-west-2';\n-  // TODO(lavelle): make this a parameter and connect to the bucket selection\n-  // UI.\n-  var bucket = 'chromeostest';\n-\n-  window.s3fs = new S3FS(key, secret, region, bucket);\n-\n-  // Register each of the event listeners to the FSP.\n-  for (var name in events) {\n-    chrome.fileSystemProvider[name].addListener(events[name]);\n+var keys = ['bucket', 'region', 'access', 'secret'];\n+\n+/**\n+ * Mounts an S3 bucket with the given name and region as a file system in the\n+ * file browser.\n+ *\n+ * @param {Object} options\n+ *     @param {string} bucket The name of the bucket.\n+ *     @param {string} region The AWS region of the bucket.\n+ *     @param {string} access The AWS access key ID for the user who is accessing\n+ *         the bucket.\n+ *     @param {string} secret The AWS secret key for the user who is accessing\n+ *         the bucket.\n+ *     @param {function} onSuccess The function to be called when the bucket is\n+ *         mounted successfully.\n+ *     @param {function} onError The function to be called if the bucket fails\n+ *         to mount;","path":"s3fs/js/main.js","position":41,"original_position":41,"commit_id":"3630ce5e846f94365a31c416aaa3951a7809acd6","original_commit_id":"50d02ba784d553440cbaa76577bbe2debecad358","user":{"login":"lavelle","id":550185,"avatar_url":"https://avatars.githubusercontent.com/u/550185?v=2","gravatar_id":"8ff364476b280cd51aba531052a0603c","url":"https://api.github.com/users/lavelle","html_url":"https://github.com/lavelle","followers_url":"https://api.github.com/users/lavelle/followers","following_url":"https://api.github.com/users/lavelle/following{/other_user}","gists_url":"https://api.github.com/users/lavelle/gists{/gist_id}","starred_url":"https://api.github.com/users/lavelle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavelle/subscriptions","organizations_url":"https://api.github.com/users/lavelle/orgs","repos_url":"https://api.github.com/users/lavelle/repos","events_url":"https://api.github.com/users/lavelle/events{/privacy}","received_events_url":"https://api.github.com/users/lavelle/received_events","type":"User","site_admin":false},"body":"I find it's super ugly to have multiple functions as arguments to a function. There's no nice way to format them. Like:\r\n\r\n```js\r\nfoo(function() {\r\n   do_something();\r\n}, function() {\r\n   do_something_else();\r\n});\r\n```\r\nlooks much worse IMO than\r\n```js\r\nfoo({\r\n  arg1: function() {\r\n    do_something();\r\n  },\r\n  arg2: function() {\r\n   do_something_else();\r\n  }\r\n});\r\n```\r\n","created_at":"2014-08-20T21:25:26Z","updated_at":"2014-08-20T21:25:26Z","html_url":"https://github.com/google/chromeos-filesystems/pull/22#discussion_r16506706","pull_request_url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22","_links":{"self":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/comments/16506706"},"html":{"href":"https://github.com/google/chromeos-filesystems/pull/22#discussion_r16506706"},"pull_request":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22"}}},"pull_request":{"url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22","id":20010894,"html_url":"https://github.com/google/chromeos-filesystems/pull/22","diff_url":"https://github.com/google/chromeos-filesystems/pull/22.diff","patch_url":"https://github.com/google/chromeos-filesystems/pull/22.patch","issue_url":"https://api.github.com/repos/google/chromeos-filesystems/issues/22","number":22,"state":"open","locked":false,"title":"Add basic UI to S3FS","user":{"login":"lavelle","id":550185,"avatar_url":"https://avatars.githubusercontent.com/u/550185?v=2","gravatar_id":"8ff364476b280cd51aba531052a0603c","url":"https://api.github.com/users/lavelle","html_url":"https://github.com/lavelle","followers_url":"https://api.github.com/users/lavelle/followers","following_url":"https://api.github.com/users/lavelle/following{/other_user}","gists_url":"https://api.github.com/users/lavelle/gists{/gist_id}","starred_url":"https://api.github.com/users/lavelle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavelle/subscriptions","organizations_url":"https://api.github.com/users/lavelle/orgs","repos_url":"https://api.github.com/users/lavelle/repos","events_url":"https://api.github.com/users/lavelle/events{/privacy}","received_events_url":"https://api.github.com/users/lavelle/received_events","type":"User","site_admin":false},"body":"This PR closes #15.\r\n\r\nThis will just create a very basic initial working implementation of the UI.\r\n\r\nOnce this is merged, enhancements can be made via future PRs:\r\n\r\n- Validation of the credentials using [AWSValidator][]\r\n- Polymer components for the input fields, button and toasts using [ToastManager][]\r\n\r\n[awsvalidator]: https://github.com/lavelle/chromeos-filesystems/blob/s3fs/s3fs/ui/js/lib/awsvalidator.js\r\n[toastmanager]: https://github.com/google/chromeos-filesystems/pull/10\r\n\r\nCurrent status:\r\n\r\n- [x] Only support one instance per provider\r\n- [x] Open a window prompting the user to enter credentials when the extension is installed\r\n- [ ] If the user enters invalid credentials, or any other error occurs, the window will reappear with an appropriate error message, until the user enters correct credentials or closes the window\r\n- [ ] If the user closes this window, the extension goes into a deactivated state\r\n- [x] The filesystem instance persists as long as the extension is installed, and remounts itself after any Chrome relaunches\r\n- [x] To unmount the filesystem instance, the user uninstalls the extension\r\n- [ ] If the credentials ever expire or change, the extension will go into a deactivated state\r\n- [ ] When in the deactivated state caused by 4 or 7, the extension will prompt the user to enter new credentials as in step 2 whenever an attempt is made to use it","created_at":"2014-08-19T21:32:02Z","updated_at":"2014-08-20T21:25:26Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9dc7978812e50b95cea39d8b4c6a24f06cc17e4e","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22/commits","review_comments_url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22/comments","review_comment_url":"https://api.github.com/repos/google/chromeos-filesystems/pulls/comments/{number}","comments_url":"https://api.github.com/repos/google/chromeos-filesystems/issues/22/comments","statuses_url":"https://api.github.com/repos/google/chromeos-filesystems/statuses/3630ce5e846f94365a31c416aaa3951a7809acd6","head":{"label":"lavelle:basic-ui","ref":"basic-ui","sha":"3630ce5e846f94365a31c416aaa3951a7809acd6","user":{"login":"lavelle","id":550185,"avatar_url":"https://avatars.githubusercontent.com/u/550185?v=2","gravatar_id":"8ff364476b280cd51aba531052a0603c","url":"https://api.github.com/users/lavelle","html_url":"https://github.com/lavelle","followers_url":"https://api.github.com/users/lavelle/followers","following_url":"https://api.github.com/users/lavelle/following{/other_user}","gists_url":"https://api.github.com/users/lavelle/gists{/gist_id}","starred_url":"https://api.github.com/users/lavelle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavelle/subscriptions","organizations_url":"https://api.github.com/users/lavelle/orgs","repos_url":"https://api.github.com/users/lavelle/repos","events_url":"https://api.github.com/users/lavelle/events{/privacy}","received_events_url":"https://api.github.com/users/lavelle/received_events","type":"User","site_admin":false},"repo":{"id":22894114,"name":"chromeos-filesystems","full_name":"lavelle/chromeos-filesystems","owner":{"login":"lavelle","id":550185,"avatar_url":"https://avatars.githubusercontent.com/u/550185?v=2","gravatar_id":"8ff364476b280cd51aba531052a0603c","url":"https://api.github.com/users/lavelle","html_url":"https://github.com/lavelle","followers_url":"https://api.github.com/users/lavelle/followers","following_url":"https://api.github.com/users/lavelle/following{/other_user}","gists_url":"https://api.github.com/users/lavelle/gists{/gist_id}","starred_url":"https://api.github.com/users/lavelle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavelle/subscriptions","organizations_url":"https://api.github.com/users/lavelle/orgs","repos_url":"https://api.github.com/users/lavelle/repos","events_url":"https://api.github.com/users/lavelle/events{/privacy}","received_events_url":"https://api.github.com/users/lavelle/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/lavelle/chromeos-filesystems","description":"Additional file system providers for Chrome OS","fork":true,"url":"https://api.github.com/repos/lavelle/chromeos-filesystems","forks_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/forks","keys_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/teams","hooks_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/hooks","issue_events_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/issues/events{/number}","events_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/events","assignees_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/assignees{/user}","branches_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/branches{/branch}","tags_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/tags","blobs_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/git/refs{/sha}","trees_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/statuses/{sha}","languages_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/languages","stargazers_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/stargazers","contributors_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/contributors","subscribers_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/subscribers","subscription_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/subscription","commits_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/commits{/sha}","git_commits_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/git/commits{/sha}","comments_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/comments{/number}","issue_comment_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/issues/comments/{number}","contents_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/contents/{+path}","compare_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/merges","archive_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/downloads","issues_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/issues{/number}","pulls_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/pulls{/number}","milestones_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/milestones{/number}","notifications_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/labels{/name}","releases_url":"https://api.github.com/repos/lavelle/chromeos-filesystems/releases{/id}","created_at":"2014-08-12T22:07:52Z","updated_at":"2014-08-12T23:37:27Z","pushed_at":"2014-08-20T21:22:32Z","git_url":"git://github.com/lavelle/chromeos-filesystems.git","ssh_url":"git@github.com:lavelle/chromeos-filesystems.git","clone_url":"https://github.com/lavelle/chromeos-filesystems.git","svn_url":"https://github.com/lavelle/chromeos-filesystems","homepage":"","size":497,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"google:master","ref":"master","sha":"ae3e67fd7e1ce1fc1045bbcb2a74a98cb819d647","user":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=2","gravatar_id":"24ba30616d2a20673f54c2aee36d159e","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":22891528,"name":"chromeos-filesystems","full_name":"google/chromeos-filesystems","owner":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=2","gravatar_id":"24ba30616d2a20673f54c2aee36d159e","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/chromeos-filesystems","description":"Additional file system providers for Chrome OS","fork":false,"url":"https://api.github.com/repos/google/chromeos-filesystems","forks_url":"https://api.github.com/repos/google/chromeos-filesystems/forks","keys_url":"https://api.github.com/repos/google/chromeos-filesystems/keys{/key_id}","collaborators_url":"https://api.github.com/repos/google/chromeos-filesystems/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/google/chromeos-filesystems/teams","hooks_url":"https://api.github.com/repos/google/chromeos-filesystems/hooks","issue_events_url":"https://api.github.com/repos/google/chromeos-filesystems/issues/events{/number}","events_url":"https://api.github.com/repos/google/chromeos-filesystems/events","assignees_url":"https://api.github.com/repos/google/chromeos-filesystems/assignees{/user}","branches_url":"https://api.github.com/repos/google/chromeos-filesystems/branches{/branch}","tags_url":"https://api.github.com/repos/google/chromeos-filesystems/tags","blobs_url":"https://api.github.com/repos/google/chromeos-filesystems/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/google/chromeos-filesystems/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/google/chromeos-filesystems/git/refs{/sha}","trees_url":"https://api.github.com/repos/google/chromeos-filesystems/git/trees{/sha}","statuses_url":"https://api.github.com/repos/google/chromeos-filesystems/statuses/{sha}","languages_url":"https://api.github.com/repos/google/chromeos-filesystems/languages","stargazers_url":"https://api.github.com/repos/google/chromeos-filesystems/stargazers","contributors_url":"https://api.github.com/repos/google/chromeos-filesystems/contributors","subscribers_url":"https://api.github.com/repos/google/chromeos-filesystems/subscribers","subscription_url":"https://api.github.com/repos/google/chromeos-filesystems/subscription","commits_url":"https://api.github.com/repos/google/chromeos-filesystems/commits{/sha}","git_commits_url":"https://api.github.com/repos/google/chromeos-filesystems/git/commits{/sha}","comments_url":"https://api.github.com/repos/google/chromeos-filesystems/comments{/number}","issue_comment_url":"https://api.github.com/repos/google/chromeos-filesystems/issues/comments/{number}","contents_url":"https://api.github.com/repos/google/chromeos-filesystems/contents/{+path}","compare_url":"https://api.github.com/repos/google/chromeos-filesystems/compare/{base}...{head}","merges_url":"https://api.github.com/repos/google/chromeos-filesystems/merges","archive_url":"https://api.github.com/repos/google/chromeos-filesystems/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/google/chromeos-filesystems/downloads","issues_url":"https://api.github.com/repos/google/chromeos-filesystems/issues{/number}","pulls_url":"https://api.github.com/repos/google/chromeos-filesystems/pulls{/number}","milestones_url":"https://api.github.com/repos/google/chromeos-filesystems/milestones{/number}","notifications_url":"https://api.github.com/repos/google/chromeos-filesystems/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/google/chromeos-filesystems/labels{/name}","releases_url":"https://api.github.com/repos/google/chromeos-filesystems/releases{/id}","created_at":"2014-08-12T20:31:22Z","updated_at":"2014-08-20T05:16:20Z","pushed_at":"2014-08-20T20:58:53Z","git_url":"git://github.com/google/chromeos-filesystems.git","ssh_url":"git@github.com:google/chromeos-filesystems.git","clone_url":"https://github.com/google/chromeos-filesystems.git","svn_url":"https://github.com/google/chromeos-filesystems","homepage":"","size":730,"stargazers_count":3,"watchers_count":3,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":1,"mirror_url":null,"open_issues_count":15,"forks":1,"open_issues":15,"watchers":3,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22"},"html":{"href":"https://github.com/google/chromeos-filesystems/pull/22"},"issue":{"href":"https://api.github.com/repos/google/chromeos-filesystems/issues/22"},"comments":{"href":"https://api.github.com/repos/google/chromeos-filesystems/issues/22/comments"},"review_comments":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22/comments"},"review_comment":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/google/chromeos-filesystems/pulls/22/commits"},"statuses":{"href":"https://api.github.com/repos/google/chromeos-filesystems/statuses/3630ce5e846f94365a31c416aaa3951a7809acd6"}}}}