{"action":"created","comment":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/comments/45921992","id":45921992,"diff_hunk":"@@ -2,66 +2,80 @@\n Core logic for Comprehensive Theming.\n \"\"\"\n \n+from os.path import basename\n+\n from django.conf import settings\n \n+from path import Path\n+\n import edxmako\n \n \n-def comprehensive_theme_changes(theme_dir):\n+def get_configuration(path_theme):","path":"openedx/core/djangoapps/theming/core.py","position":14,"original_position":14,"commit_id":"a968ae67b30e0d8e7d2096165a02ed18725df358","original_commit_id":"a968ae67b30e0d8e7d2096165a02ed18725df358","user":{"login":"stvstnfrd","id":7300975,"avatar_url":"https://avatars.githubusercontent.com/u/7300975?v=3","gravatar_id":"","url":"https://api.github.com/users/stvstnfrd","html_url":"https://github.com/stvstnfrd","followers_url":"https://api.github.com/users/stvstnfrd/followers","following_url":"https://api.github.com/users/stvstnfrd/following{/other_user}","gists_url":"https://api.github.com/users/stvstnfrd/gists{/gist_id}","starred_url":"https://api.github.com/users/stvstnfrd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stvstnfrd/subscriptions","organizations_url":"https://api.github.com/users/stvstnfrd/orgs","repos_url":"https://api.github.com/users/stvstnfrd/repos","events_url":"https://api.github.com/users/stvstnfrd/events{/privacy}","received_events_url":"https://api.github.com/users/stvstnfrd/received_events","type":"User","site_admin":false},"body":"Another generalization. We don't need `theme` in the method name; it's already in the name of the module.\r\n\r\n\"changes\" itself is a bit ambiguous. I know that what the implementation currently does with this result is \"change\" something with it, but the method itself is simply returning the configuration required. \"change\" implies a delta, where \"configuration\" does not.\r\n\r\nI also prefer verbs in method names :)","created_at":"2015-11-25T21:38:20Z","updated_at":"2015-11-25T21:38:20Z","html_url":"https://github.com/edx/edx-platform/pull/10746#discussion_r45921992","pull_request_url":"https://api.github.com/repos/edx/edx-platform/pulls/10746","_links":{"self":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/comments/45921992"},"html":{"href":"https://github.com/edx/edx-platform/pull/10746#discussion_r45921992"},"pull_request":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/10746"}}},"pull_request":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/10746","id":51847662,"html_url":"https://github.com/edx/edx-platform/pull/10746","diff_url":"https://github.com/edx/edx-platform/pull/10746.diff","patch_url":"https://github.com/edx/edx-platform/pull/10746.patch","issue_url":"https://api.github.com/repos/edx/edx-platform/issues/10746","number":10746,"state":"open","locked":false,"title":"[WIP] Review theming","user":{"login":"stvstnfrd","id":7300975,"avatar_url":"https://avatars.githubusercontent.com/u/7300975?v=3","gravatar_id":"","url":"https://api.github.com/users/stvstnfrd","html_url":"https://github.com/stvstnfrd","followers_url":"https://api.github.com/users/stvstnfrd/followers","following_url":"https://api.github.com/users/stvstnfrd/following{/other_user}","gists_url":"https://api.github.com/users/stvstnfrd/gists{/gist_id}","starred_url":"https://api.github.com/users/stvstnfrd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stvstnfrd/subscriptions","organizations_url":"https://api.github.com/users/stvstnfrd/orgs","repos_url":"https://api.github.com/users/stvstnfrd/repos","events_url":"https://api.github.com/users/stvstnfrd/events{/privacy}","received_events_url":"https://api.github.com/users/stvstnfrd/received_events","type":"User","site_admin":false},"body":"@singingwolfboy In the spirit of \"show don't tell\", I offer my initial\r\nreview of the code in the form of a PR. Some of it is direct code\r\nchanges (it was easier to just do it myself than to request it of you),\r\nas well as in-line comments/questions.\r\n\r\nI'll go through this and curate it a bit more with PR comments,\r\nproviding any context that may be otherwise missing.\r\n\r\nI would suggest the best approach to digesting this will be to review\r\nthe commits one-by-one; the commit messages provide addditonal context\r\nand are generally well-separated.\r\n\r\nWhile this addresses my inital interpretation of the code, I do have\r\nsome outstanding questions/comments/concerns about the feature on a\r\nhigher level. I'm typing that up now and will follow-up shortly.\r\n\r\nI'm sure they'll be push-back, so let me know..\r\n","created_at":"2015-11-25T21:24:02Z","updated_at":"2015-11-25T21:38:20Z","closed_at":null,"merged_at":null,"merge_commit_sha":"df256eee9b402e4dcff42bfed2a79bf05179ea25","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/edx/edx-platform/pulls/10746/commits","review_comments_url":"https://api.github.com/repos/edx/edx-platform/pulls/10746/comments","review_comment_url":"https://api.github.com/repos/edx/edx-platform/pulls/comments{/number}","comments_url":"https://api.github.com/repos/edx/edx-platform/issues/10746/comments","statuses_url":"https://api.github.com/repos/edx/edx-platform/statuses/a968ae67b30e0d8e7d2096165a02ed18725df358","head":{"label":"stvstnfrd:comprehensive-theming","ref":"comprehensive-theming","sha":"a968ae67b30e0d8e7d2096165a02ed18725df358","user":{"login":"stvstnfrd","id":7300975,"avatar_url":"https://avatars.githubusercontent.com/u/7300975?v=3","gravatar_id":"","url":"https://api.github.com/users/stvstnfrd","html_url":"https://github.com/stvstnfrd","followers_url":"https://api.github.com/users/stvstnfrd/followers","following_url":"https://api.github.com/users/stvstnfrd/following{/other_user}","gists_url":"https://api.github.com/users/stvstnfrd/gists{/gist_id}","starred_url":"https://api.github.com/users/stvstnfrd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stvstnfrd/subscriptions","organizations_url":"https://api.github.com/users/stvstnfrd/orgs","repos_url":"https://api.github.com/users/stvstnfrd/repos","events_url":"https://api.github.com/users/stvstnfrd/events{/privacy}","received_events_url":"https://api.github.com/users/stvstnfrd/received_events","type":"User","site_admin":false},"repo":{"id":21055408,"name":"edx-platform","full_name":"stvstnfrd/edx-platform","owner":{"login":"stvstnfrd","id":7300975,"avatar_url":"https://avatars.githubusercontent.com/u/7300975?v=3","gravatar_id":"","url":"https://api.github.com/users/stvstnfrd","html_url":"https://github.com/stvstnfrd","followers_url":"https://api.github.com/users/stvstnfrd/followers","following_url":"https://api.github.com/users/stvstnfrd/following{/other_user}","gists_url":"https://api.github.com/users/stvstnfrd/gists{/gist_id}","starred_url":"https://api.github.com/users/stvstnfrd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stvstnfrd/subscriptions","organizations_url":"https://api.github.com/users/stvstnfrd/orgs","repos_url":"https://api.github.com/users/stvstnfrd/repos","events_url":"https://api.github.com/users/stvstnfrd/events{/privacy}","received_events_url":"https://api.github.com/users/stvstnfrd/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/stvstnfrd/edx-platform","description":"the edX learning management system (LMS) and course authoring tool, Studio","fork":true,"url":"https://api.github.com/repos/stvstnfrd/edx-platform","forks_url":"https://api.github.com/repos/stvstnfrd/edx-platform/forks","keys_url":"https://api.github.com/repos/stvstnfrd/edx-platform/keys{/key_id}","collaborators_url":"https://api.github.com/repos/stvstnfrd/edx-platform/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/stvstnfrd/edx-platform/teams","hooks_url":"https://api.github.com/repos/stvstnfrd/edx-platform/hooks","issue_events_url":"https://api.github.com/repos/stvstnfrd/edx-platform/issues/events{/number}","events_url":"https://api.github.com/repos/stvstnfrd/edx-platform/events","assignees_url":"https://api.github.com/repos/stvstnfrd/edx-platform/assignees{/user}","branches_url":"https://api.github.com/repos/stvstnfrd/edx-platform/branches{/branch}","tags_url":"https://api.github.com/repos/stvstnfrd/edx-platform/tags","blobs_url":"https://api.github.com/repos/stvstnfrd/edx-platform/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/stvstnfrd/edx-platform/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/stvstnfrd/edx-platform/git/refs{/sha}","trees_url":"https://api.github.com/repos/stvstnfrd/edx-platform/git/trees{/sha}","statuses_url":"https://api.github.com/repos/stvstnfrd/edx-platform/statuses/{sha}","languages_url":"https://api.github.com/repos/stvstnfrd/edx-platform/languages","stargazers_url":"https://api.github.com/repos/stvstnfrd/edx-platform/stargazers","contributors_url":"https://api.github.com/repos/stvstnfrd/edx-platform/contributors","subscribers_url":"https://api.github.com/repos/stvstnfrd/edx-platform/subscribers","subscription_url":"https://api.github.com/repos/stvstnfrd/edx-platform/subscription","commits_url":"https://api.github.com/repos/stvstnfrd/edx-platform/commits{/sha}","git_commits_url":"https://api.github.com/repos/stvstnfrd/edx-platform/git/commits{/sha}","comments_url":"https://api.github.com/repos/stvstnfrd/edx-platform/comments{/number}","issue_comment_url":"https://api.github.com/repos/stvstnfrd/edx-platform/issues/comments{/number}","contents_url":"https://api.github.com/repos/stvstnfrd/edx-platform/contents/{+path}","compare_url":"https://api.github.com/repos/stvstnfrd/edx-platform/compare/{base}...{head}","merges_url":"https://api.github.com/repos/stvstnfrd/edx-platform/merges","archive_url":"https://api.github.com/repos/stvstnfrd/edx-platform/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/stvstnfrd/edx-platform/downloads","issues_url":"https://api.github.com/repos/stvstnfrd/edx-platform/issues{/number}","pulls_url":"https://api.github.com/repos/stvstnfrd/edx-platform/pulls{/number}","milestones_url":"https://api.github.com/repos/stvstnfrd/edx-platform/milestones{/number}","notifications_url":"https://api.github.com/repos/stvstnfrd/edx-platform/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/stvstnfrd/edx-platform/labels{/name}","releases_url":"https://api.github.com/repos/stvstnfrd/edx-platform/releases{/id}","created_at":"2014-06-20T23:21:58Z","updated_at":"2015-03-18T19:16:49Z","pushed_at":"2015-11-25T21:13:45Z","git_url":"git://github.com/stvstnfrd/edx-platform.git","ssh_url":"git@github.com:stvstnfrd/edx-platform.git","clone_url":"https://github.com/stvstnfrd/edx-platform.git","svn_url":"https://github.com/stvstnfrd/edx-platform","homepage":"http://code.edx.org/","size":490989,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":0,"mirror_url":null,"open_issues_count":6,"forks":0,"open_issues":6,"watchers":0,"default_branch":"master"}},"base":{"label":"edx:db/edx.org-comp-theme","ref":"db/edx.org-comp-theme","sha":"ae3f2c672c853cba00086b7441d0f4c387597901","user":{"login":"edx","id":3179841,"avatar_url":"https://avatars.githubusercontent.com/u/3179841?v=3","gravatar_id":"","url":"https://api.github.com/users/edx","html_url":"https://github.com/edx","followers_url":"https://api.github.com/users/edx/followers","following_url":"https://api.github.com/users/edx/following{/other_user}","gists_url":"https://api.github.com/users/edx/gists{/gist_id}","starred_url":"https://api.github.com/users/edx/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edx/subscriptions","organizations_url":"https://api.github.com/users/edx/orgs","repos_url":"https://api.github.com/users/edx/repos","events_url":"https://api.github.com/users/edx/events{/privacy}","received_events_url":"https://api.github.com/users/edx/received_events","type":"Organization","site_admin":false},"repo":{"id":10391073,"name":"edx-platform","full_name":"edx/edx-platform","owner":{"login":"edx","id":3179841,"avatar_url":"https://avatars.githubusercontent.com/u/3179841?v=3","gravatar_id":"","url":"https://api.github.com/users/edx","html_url":"https://github.com/edx","followers_url":"https://api.github.com/users/edx/followers","following_url":"https://api.github.com/users/edx/following{/other_user}","gists_url":"https://api.github.com/users/edx/gists{/gist_id}","starred_url":"https://api.github.com/users/edx/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edx/subscriptions","organizations_url":"https://api.github.com/users/edx/orgs","repos_url":"https://api.github.com/users/edx/repos","events_url":"https://api.github.com/users/edx/events{/privacy}","received_events_url":"https://api.github.com/users/edx/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/edx/edx-platform","description":"The Open edX platform, the software that powers edX!","fork":false,"url":"https://api.github.com/repos/edx/edx-platform","forks_url":"https://api.github.com/repos/edx/edx-platform/forks","keys_url":"https://api.github.com/repos/edx/edx-platform/keys{/key_id}","collaborators_url":"https://api.github.com/repos/edx/edx-platform/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/edx/edx-platform/teams","hooks_url":"https://api.github.com/repos/edx/edx-platform/hooks","issue_events_url":"https://api.github.com/repos/edx/edx-platform/issues/events{/number}","events_url":"https://api.github.com/repos/edx/edx-platform/events","assignees_url":"https://api.github.com/repos/edx/edx-platform/assignees{/user}","branches_url":"https://api.github.com/repos/edx/edx-platform/branches{/branch}","tags_url":"https://api.github.com/repos/edx/edx-platform/tags","blobs_url":"https://api.github.com/repos/edx/edx-platform/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/edx/edx-platform/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/edx/edx-platform/git/refs{/sha}","trees_url":"https://api.github.com/repos/edx/edx-platform/git/trees{/sha}","statuses_url":"https://api.github.com/repos/edx/edx-platform/statuses/{sha}","languages_url":"https://api.github.com/repos/edx/edx-platform/languages","stargazers_url":"https://api.github.com/repos/edx/edx-platform/stargazers","contributors_url":"https://api.github.com/repos/edx/edx-platform/contributors","subscribers_url":"https://api.github.com/repos/edx/edx-platform/subscribers","subscription_url":"https://api.github.com/repos/edx/edx-platform/subscription","commits_url":"https://api.github.com/repos/edx/edx-platform/commits{/sha}","git_commits_url":"https://api.github.com/repos/edx/edx-platform/git/commits{/sha}","comments_url":"https://api.github.com/repos/edx/edx-platform/comments{/number}","issue_comment_url":"https://api.github.com/repos/edx/edx-platform/issues/comments{/number}","contents_url":"https://api.github.com/repos/edx/edx-platform/contents/{+path}","compare_url":"https://api.github.com/repos/edx/edx-platform/compare/{base}...{head}","merges_url":"https://api.github.com/repos/edx/edx-platform/merges","archive_url":"https://api.github.com/repos/edx/edx-platform/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/edx/edx-platform/downloads","issues_url":"https://api.github.com/repos/edx/edx-platform/issues{/number}","pulls_url":"https://api.github.com/repos/edx/edx-platform/pulls{/number}","milestones_url":"https://api.github.com/repos/edx/edx-platform/milestones{/number}","notifications_url":"https://api.github.com/repos/edx/edx-platform/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/edx/edx-platform/labels{/name}","releases_url":"https://api.github.com/repos/edx/edx-platform/releases{/id}","created_at":"2013-05-30T20:20:38Z","updated_at":"2015-11-25T18:57:02Z","pushed_at":"2015-11-25T21:37:39Z","git_url":"git://github.com/edx/edx-platform.git","ssh_url":"git@github.com:edx/edx-platform.git","clone_url":"https://github.com/edx/edx-platform.git","svn_url":"https://github.com/edx/edx-platform","homepage":"http://open.edx.org/","size":626050,"stargazers_count":2258,"watchers_count":2258,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1384,"mirror_url":null,"open_issues_count":157,"forks":1384,"open_issues":157,"watchers":2258,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/10746"},"html":{"href":"https://github.com/edx/edx-platform/pull/10746"},"issue":{"href":"https://api.github.com/repos/edx/edx-platform/issues/10746"},"comments":{"href":"https://api.github.com/repos/edx/edx-platform/issues/10746/comments"},"review_comments":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/10746/comments"},"review_comment":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/10746/commits"},"statuses":{"href":"https://api.github.com/repos/edx/edx-platform/statuses/a968ae67b30e0d8e7d2096165a02ed18725df358"}}}}