{"action":"created","issue":{"url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/622","repository_url":"https://api.github.com/repos/ocadotechnology/aimmo","labels_url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/622/labels{/name}","comments_url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/622/comments","events_url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/622/events","html_url":"https://github.com/ocadotechnology/aimmo/pull/622","id":332352044,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0ODU0MTk2","number":622,"title":"Add only school users to game","user":{"login":"riaJha97","id":37871616,"node_id":"MDQ6VXNlcjM3ODcxNjE2","avatar_url":"https://avatars3.githubusercontent.com/u/37871616?v=4","gravatar_id":"","url":"https://api.github.com/users/riaJha97","html_url":"https://github.com/riaJha97","followers_url":"https://api.github.com/users/riaJha97/followers","following_url":"https://api.github.com/users/riaJha97/following{/other_user}","gists_url":"https://api.github.com/users/riaJha97/gists{/gist_id}","starred_url":"https://api.github.com/users/riaJha97/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/riaJha97/subscriptions","organizations_url":"https://api.github.com/users/riaJha97/orgs","repos_url":"https://api.github.com/users/riaJha97/repos","events_url":"https://api.github.com/users/riaJha97/events{/privacy}","received_events_url":"https://api.github.com/users/riaJha97/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2018-06-14T10:56:37Z","updated_at":"2018-06-18T10:57:51Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/ocadotechnology/aimmo/pulls/622","html_url":"https://github.com/ocadotechnology/aimmo/pull/622","diff_url":"https://github.com/ocadotechnology/aimmo/pull/622.diff","patch_url":"https://github.com/ocadotechnology/aimmo/pull/622.patch"},"body":"Must be merged before the pull request on portal is merged into master\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/ocadotechnology/aimmo/622)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/comments/398017430","html_url":"https://github.com/ocadotechnology/aimmo/pull/622#issuecomment-398017430","issue_url":"https://api.github.com/repos/ocadotechnology/aimmo/issues/622","id":398017430,"node_id":"MDEyOklzc3VlQ29tbWVudDM5ODAxNzQzMA==","user":{"login":"mrniket","id":2016308,"node_id":"MDQ6VXNlcjIwMTYzMDg=","avatar_url":"https://avatars2.githubusercontent.com/u/2016308?v=4","gravatar_id":"","url":"https://api.github.com/users/mrniket","html_url":"https://github.com/mrniket","followers_url":"https://api.github.com/users/mrniket/followers","following_url":"https://api.github.com/users/mrniket/following{/other_user}","gists_url":"https://api.github.com/users/mrniket/gists{/gist_id}","starred_url":"https://api.github.com/users/mrniket/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mrniket/subscriptions","organizations_url":"https://api.github.com/users/mrniket/orgs","repos_url":"https://api.github.com/users/mrniket/repos","events_url":"https://api.github.com/users/mrniket/events{/privacy}","received_events_url":"https://api.github.com/users/mrniket/received_events","type":"User","site_admin":false},"created_at":"2018-06-18T10:57:51Z","updated_at":"2018-06-18T10:57:51Z","author_association":"MEMBER","body":"\n\n\n\nReviewed 1 of 8 files at r2, 6 of 18 files at r3, 3 of 13 files at r5, 1 of 2 files at r6, 2 of 4 files at r7.\nReview status: all files reviewed, 6 unresolved discussions (waiting on @OlafSzmidt and @riaJha97)\n\n---\n\n*[all_tests.py, line 17 at r7](https://reviewable.io/reviews/ocadotechnology/aimmo/622#-LFHV9-YCtSDfJ00Ep8l:-LFHV9-YCtSDfJ00Ep8m:bd72arx) ([raw file](https://github.com/ocadotechnology/aimmo/blob/24fb51d01c7f6b4541c55ec7a92898617658cd09/all_tests.py#L17)):*\n> ```Python\n> \n> BASE_DIR = os.path.abspath(os.path.dirname(__file__))\n> APPS = ('', 'aimmo-game/', 'aimmo-game-worker/', 'aimmo-game-creator/',)\n> ```\n\ninteresting the trailing comma doesn't break anything!\n\n---\n\n*[aimmo_runner/runner.py, line 38 at r7](https://reviewable.io/reviews/ocadotechnology/aimmo/622#-LFHVL8p6m6SAWw3Adbd:-LFHVL8p6m6SAWw3Adbe:b-dxj2zy) ([raw file](https://github.com/ocadotechnology/aimmo/blob/24fb51d01c7f6b4541c55ec7a92898617658cd09/aimmo_runner/runner.py#L38)):*\n> ```Python\n> def run(use_minikube, server_wait=True, capture_output=False, test_env=False):\n>     logging.basicConfig()\n>     if test_env:\n> ```\n\nis the test_env for local development or for testing?\n\n---\n\n*[players/app_settings.py, line 23 at r6](https://reviewable.io/reviews/ocadotechnology/aimmo/622#-LExzpPU6wiDndIgJN5v:-LFHWg0F7zr5AuZI3udz:b9pw2tj) ([raw file](https://github.com/ocadotechnology/aimmo/blob/22576cfa0ab73e70901d469a617d98623de0c23d/players/app_settings.py#L23)):*\n<details><summary><i>Previously, riaJha97 wrote…</i></summary><blockquote>\n\nI've changed the function name but I've been following rapid router so I'm not sure where it should go\n</blockquote></details>\n\neither a decorators folder or maybe we can have a config file/folder somewhere. I say config because it makes it explicit that that's what this is. Then again this is the app_settings file so maybe this is the config file that rapid router chose. I guess we can just make it better organised \n\n(sorry internal monologue here)\n\n---\n\n*[players/app_settings.py, line 28 at r7](https://reviewable.io/reviews/ocadotechnology/aimmo/622#-LFHJ6wO0NBgyA_901eX:-LFHW4hM5peeFKKpGl3K:b-kxp6rx) ([raw file](https://github.com/ocadotechnology/aimmo/blob/24fb51d01c7f6b4541c55ec7a92898617658cd09/players/app_settings.py#L28)):*\n<details><summary><i>Previously, OlafSzmidt (Olaf Szmidt) wrote…</i></summary><blockquote>\n\n> ```\n> preview_user = get_aimmo_preview_user_decorator()\n> ```\n\nWould be good if others (@mrniket, @celineboudier) would comment on this, but what if we had a `decorators` file somewhere and instead of doing this, we would just have decorator functions imported directly from that file in places where they're being used?\n</blockquote></details>\n\nyeah, a decorators folder or file would work, in portal we have custom decorators too right?\n\n---\n\n*[players/app_settings.py, line 28 at r7](https://reviewable.io/reviews/ocadotechnology/aimmo/622#-LFHXXxRCWG8xA8efz4Y:-LFHXXxRCWG8xA8efz4Z:btdge2e) ([raw file](https://github.com/ocadotechnology/aimmo/blob/24fb51d01c7f6b4541c55ec7a92898617658cd09/players/app_settings.py#L28)):*\n> ```Python\n> \n> \n> preview_user = get_aimmo_preview_user_decorator()\n> ```\n\nMight be good to rename this `preview_user_required` so it matches the convention of the `login_required` decorator?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io/reviews/ocadotechnology/aimmo/622)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"mrniket"}}