{"action":"created","issue":{"url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/798","repository_url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf","labels_url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/798/labels{/name}","comments_url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/798/comments","events_url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/798/events","html_url":"https://github.com/fgpv-vpgf/fgpv-vpgf/pull/798","id":161215699,"number":798,"title":"Bookmarking","user":{"login":"spencerwahl","id":8732588,"avatar_url":"https://avatars.githubusercontent.com/u/8732588?v=3","gravatar_id":"","url":"https://api.github.com/users/spencerwahl","html_url":"https://github.com/spencerwahl","followers_url":"https://api.github.com/users/spencerwahl/followers","following_url":"https://api.github.com/users/spencerwahl/following{/other_user}","gists_url":"https://api.github.com/users/spencerwahl/gists{/gist_id}","starred_url":"https://api.github.com/users/spencerwahl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spencerwahl/subscriptions","organizations_url":"https://api.github.com/users/spencerwahl/orgs","repos_url":"https://api.github.com/users/spencerwahl/repos","events_url":"https://api.github.com/users/spencerwahl/events{/privacy}","received_events_url":"https://api.github.com/users/spencerwahl/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"cynngah","id":9622589,"avatar_url":"https://avatars.githubusercontent.com/u/9622589?v=3","gravatar_id":"","url":"https://api.github.com/users/cynngah","html_url":"https://github.com/cynngah","followers_url":"https://api.github.com/users/cynngah/followers","following_url":"https://api.github.com/users/cynngah/following{/other_user}","gists_url":"https://api.github.com/users/cynngah/gists{/gist_id}","starred_url":"https://api.github.com/users/cynngah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cynngah/subscriptions","organizations_url":"https://api.github.com/users/cynngah/orgs","repos_url":"https://api.github.com/users/cynngah/repos","events_url":"https://api.github.com/users/cynngah/events{/privacy}","received_events_url":"https://api.github.com/users/cynngah/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2016-06-20T14:58:12Z","updated_at":"2016-06-20T16:11:54Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/pulls/798","html_url":"https://github.com/fgpv-vpgf/fgpv-vpgf/pull/798","diff_url":"https://github.com/fgpv-vpgf/fgpv-vpgf/pull/798.diff","patch_url":"https://github.com/fgpv-vpgf/fgpv-vpgf/pull/798.patch"},"body":"\n\n<!-- Reviewable:start -->\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"35\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/fgpv-vpgf/fgpv-vpgf/798)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/comments/227189914","html_url":"https://github.com/fgpv-vpgf/fgpv-vpgf/pull/798#issuecomment-227189914","issue_url":"https://api.github.com/repos/fgpv-vpgf/fgpv-vpgf/issues/798","id":227189914,"user":{"login":"alyec","id":7646414,"avatar_url":"https://avatars.githubusercontent.com/u/7646414?v=3","gravatar_id":"","url":"https://api.github.com/users/alyec","html_url":"https://github.com/alyec","followers_url":"https://api.github.com/users/alyec/followers","following_url":"https://api.github.com/users/alyec/following{/other_user}","gists_url":"https://api.github.com/users/alyec/gists{/gist_id}","starred_url":"https://api.github.com/users/alyec/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alyec/subscriptions","organizations_url":"https://api.github.com/users/alyec/orgs","repos_url":"https://api.github.com/users/alyec/repos","events_url":"https://api.github.com/users/alyec/events{/privacy}","received_events_url":"https://api.github.com/users/alyec/received_events","type":"User","site_admin":false},"created_at":"2016-06-20T16:11:54Z","updated_at":"2016-06-20T16:11:54Z","body":"\n\n\n\nReviewed 18 of 18 files at r1.\nReview status: all files reviewed at latest revision, 8 unresolved discussions.\n\n---\n\n*[src/app/core/bookmark.service.js, line 9 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiSir_vrdrPHl5HI68:-KKiSir_vrdrPHl5HI69:-1401187647) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/bookmark.service.js#L9)):*\n> ```JavaScript\n>      *\n>      * @description\n>      *\n> ```\n\nExcellent description.\n\n---\n\n*[src/app/core/bookmark.service.js, line 47 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiYBP3nRpr7-PRYL47:-KKiYBP44trWUg7PSsFA:-703523673) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/bookmark.service.js#L47)):*\n> ```JavaScript\n>             });\n> \n>             const bookmark =  basemap + ',' + extent.x + ',' + extent.y + ',' + extent.zoom +\n> ```\n\nThis seems like a good candidate for ES6 template strings.\n\n---\n\n*[src/app/core/bookmark.service.js, line 70 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiZI_EBxPgHkM7MixY:-KKiZI_EBxPgHkM7MixZ:588520051) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/bookmark.service.js#L70)):*\n> ```JavaScript\n>             const info = bookmark.match(pattern);\n> \n>             const basemap = decode64(info[1]);\n> ```\n\n`const [basemap, x, y, zoom] = [1,2,3,4].map(i => decode64(info[i]));` ?\n\n---\n\n*[src/app/core/core.run.js, line 45 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiPq4ydzENeHW6cYil:-KKiPq4ydzENeHW6cYim:-463942294) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/core.run.js#L45)):*\n> ```JavaScript\n>         /********************/\n> \n>         function bookmarkCallback(config) {\n> ```\n\njsdoc\n\n---\n\n*[src/app/core/core.run.js, line 46 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiOjnAG4UPo_IpLjNB:-KKiOjnAG4UPo_IpLjNC:186539474) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/core.run.js#L46)):*\n> ```JavaScript\n> \n>         function bookmarkCallback(config) {\n>         const bookmarkAttr = $rootElement.attr('rv-restore-bookmark');\n> ```\n\nindentation seems to be off, but jscs didn't complain?\n\n---\n\n*[src/app/core/core.run.js, line 116 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiPx7axbHVbXG1KwEU:-KKiPx7axbHVbXG1KwEV:-2139152306) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/core/core.run.js#L116)):*\n> ```JavaScript\n>         }\n> \n>         function getBookmark() {\n> ```\n\njsdoc (also on centerAndZoom)\n\n---\n\n*[src/app/geo/layer-record.class.js, line 107 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiSB2TsLnnQ3uLXJyp:-KKiSB2TsLnnQ3uLXJyq:-1942132919) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/geo/layer-record.class.js#L107)):*\n> ```JavaScript\n>             }\n> \n>             makeLayerBookmark () { throw new Error('This should be overridden in subclasses'); }\n> ```\n\nI realize most of this file wasn't `jsdoc`ed initially (mostly because I wasn't sure if this version would get through the PR).  But we should start now (feel free to doc only the top level functions and use `@see` liberally in subclasses)\n\n---\n\n*[src/app/geo/layer-record.class.js, line 121 \\[r1\\]](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798#-KKiQd6Jnat1pX38eNUE:-KKiQd6Jnat1pX38eNUF:-109153589) ([raw file](https://github.com/fgpv-vpgf/fgpv-vpgf/blob/74e7cbbe03162b328a5474984a4c588f333f99ab/src/app/geo/layer-record.class.js#L121)):*\n> ```JavaScript\n> \n>                 props.forEach((prop, index) => {\n>                     result.options[prop] = {};\n> ```\n\n`result.options[prop] = { value: lookup[prop](info[index]) };` ?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/fgpv-vpgf/fgpv-vpgf/798)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"alyec"}}