GitHub PullRequestReviewCommentEvent at 2014-08-12 13:07:23
Repository: liquid
Actor: jasonroelofs
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/Shopify/liquid/pulls/comments/16111168","id":16111168,"diff_hunk":"@@ -133,4 +133,9 @@ def test_strict_filter_argument_parsing\n       end\n     end\n   end\n+\n+  def test_output_raw_source_of_variable","path":"test/unit/variable_unit_test.rb","position":5,"original_position":5,"commit_id":"dcc6ca1aa198e6df57da7aeb0c5f92e336fc481a","original_commit_id":"dcc6ca1aa198e6df57da7aeb0c5f92e336fc481a","user":{"login":"jasonroelofs","id":3223,"avatar_url":"https://avatars.githubusercontent.com/u/3223?v=2","gravatar_id":"2ede8c8483953513418849ba0c68278e","url":"https://api.github.com/users/jasonroelofs","html_url":"https://github.com/jasonroelofs","followers_url":"https://api.github.com/users/jasonroelofs/followers","following_url":"https://api.github.com/users/jasonroelofs/following{/other_user}","gists_url":"https://api.github.com/users/jasonroelofs/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonroelofs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonroelofs/subscriptions","organizations_url":"https://api.github.com/users/jasonroelofs/orgs","repos_url":"https://api.github.com/users/jasonroelofs/repos","events_url":"https://api.github.com/users/jasonroelofs/events{/privacy}","received_events_url":"https://api.github.com/users/jasonroelofs/received_events","type":"User","site_admin":false},"body":"The profiler results allows you to output the token and the token's location (template and line number) to allow for easy mapping back to the original template. As each token type handles it's own parsing, it made sense to me for each token type to know how to reconstitute itself as well.","created_at":"2014-08-12T13:07:23Z","updated_at":"2014-08-12T13:07:23Z","html_url":"https://github.com/Shopify/liquid/pull/364#discussion_r16111168","pull_request_url":"https://api.github.com/repos/Shopify/liquid/pulls/364","_links":{"self":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/comments/16111168"},"html":{"href":"https://github.com/Shopify/liquid/pull/364#discussion_r16111168"},"pull_request":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/364"}}},"pull_request":{"url":"https://api.github.com/repos/Shopify/liquid/pulls/364","id":16917849,"html_url":"https://github.com/Shopify/liquid/pull/364","diff_url":"https://github.com/Shopify/liquid/pull/364.diff","patch_url":"https://github.com/Shopify/liquid/pull/364.patch","issue_url":"https://api.github.com/repos/Shopify/liquid/issues/364","number":364,"state":"open","title":"Profiling the rendering of a liquid template","user":{"login":"jasonroelofs","id":3223,"avatar_url":"https://avatars.githubusercontent.com/u/3223?v=2","gravatar_id":"2ede8c8483953513418849ba0c68278e","url":"https://api.github.com/users/jasonroelofs","html_url":"https://github.com/jasonroelofs","followers_url":"https://api.github.com/users/jasonroelofs/followers","following_url":"https://api.github.com/users/jasonroelofs/following{/other_user}","gists_url":"https://api.github.com/users/jasonroelofs/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonroelofs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonroelofs/subscriptions","organizations_url":"https://api.github.com/users/jasonroelofs/orgs","repos_url":"https://api.github.com/users/jasonroelofs/repos","events_url":"https://api.github.com/users/jasonroelofs/events{/privacy}","received_events_url":"https://api.github.com/users/jasonroelofs/received_events","type":"User","site_admin":false},"body":"This is an improved version of what I originally put together in #361\r\n\r\nThis PR adds a simple profiling system to liquid rendering. Each liquid tag ({{ }} and {% %}) is processed through this profiling, keeping track of the partial name (in the case of {% include %}), line number, and the time it took to render the tag. In the case of {% include %} and any tag that requires blocks (like {% if %} and {% for %}), the tags inside of the partials and blocks are themselves marked as children in the final profiling structure, letting any user of this information view top level information and to dive down into more details. With this, it's now possible to track down exactly which tags are taking a long time to render.\r\n\r\nIncluded in this PR are internal changes to parsing to calculate and keep track of the line numbers of individual tags and tokens in the source, as well as some specific tracking of the current partial, such that using the include tag will be tracked as a nested render. I initially looked into adding this information to error exceptions but that was proving more complicated and would require quite a few changes to how tags handle syntax errors.\r\n\r\nI was able to hook profiling into Liquid through one small refactoring (`Block#render_token`) and alias method chain-like hooks into the appropriate classes, significantly reducing the code impact on the core Liquid code base. There are also changes to parsing to figure out the line numbers of the tokens, Template to start up a profiling pass, and to Tags and Variables for easier output of the contents of these items when printing out the profiled information, but otherwise the actual profiling is kept separate.  The hooks install and uninstall themselves as needed, which ensures that if profiling is turned off that there is no performance impact.\r\n\r\nThe current profiling runs for these changes can be found here: https://gist.github.com/jasonroelofs/79bde3d22ede168a5ae8\r\n\r\nI've also put together an example template and resulting profiling output here: https://gist.github.com/jasonroelofs/c8148317ffb09e541633\r\n\r\nIf you prefer not to have this in the core I would love to discuss what changes Liquid does need for this functionality to live as its own gem and hooking into Liquid when needed.","created_at":"2014-06-10T01:20:47Z","updated_at":"2014-08-12T13:07:23Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1c3a1fad2823a31631b7373ab1db4dd022b00d8d","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/Shopify/liquid/pulls/364/commits","review_comments_url":"https://api.github.com/repos/Shopify/liquid/pulls/364/comments","review_comment_url":"https://api.github.com/repos/Shopify/liquid/pulls/comments/{number}","comments_url":"https://api.github.com/repos/Shopify/liquid/issues/364/comments","statuses_url":"https://api.github.com/repos/Shopify/liquid/statuses/dcc6ca1aa198e6df57da7aeb0c5f92e336fc481a","head":{"label":"collectiveidea:instrument-rendering-with-hooks","ref":"instrument-rendering-with-hooks","sha":"dcc6ca1aa198e6df57da7aeb0c5f92e336fc481a","user":{"login":"collectiveidea","id":128,"avatar_url":"https://avatars.githubusercontent.com/u/128?v=2","gravatar_id":"13ff8dc8c2bf2a4752816e1e3f201a05","url":"https://api.github.com/users/collectiveidea","html_url":"https://github.com/collectiveidea","followers_url":"https://api.github.com/users/collectiveidea/followers","following_url":"https://api.github.com/users/collectiveidea/following{/other_user}","gists_url":"https://api.github.com/users/collectiveidea/gists{/gist_id}","starred_url":"https://api.github.com/users/collectiveidea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/collectiveidea/subscriptions","organizations_url":"https://api.github.com/users/collectiveidea/orgs","repos_url":"https://api.github.com/users/collectiveidea/repos","events_url":"https://api.github.com/users/collectiveidea/events{/privacy}","received_events_url":"https://api.github.com/users/collectiveidea/received_events","type":"Organization","site_admin":false},"repo":{"id":20112770,"name":"liquid","full_name":"collectiveidea/liquid","owner":{"login":"collectiveidea","id":128,"avatar_url":"https://avatars.githubusercontent.com/u/128?v=2","gravatar_id":"13ff8dc8c2bf2a4752816e1e3f201a05","url":"https://api.github.com/users/collectiveidea","html_url":"https://github.com/collectiveidea","followers_url":"https://api.github.com/users/collectiveidea/followers","following_url":"https://api.github.com/users/collectiveidea/following{/other_user}","gists_url":"https://api.github.com/users/collectiveidea/gists{/gist_id}","starred_url":"https://api.github.com/users/collectiveidea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/collectiveidea/subscriptions","organizations_url":"https://api.github.com/users/collectiveidea/orgs","repos_url":"https://api.github.com/users/collectiveidea/repos","events_url":"https://api.github.com/users/collectiveidea/events{/privacy}","received_events_url":"https://api.github.com/users/collectiveidea/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/collectiveidea/liquid","description":"Liquid markup language. Safe, customer facing template language for flexible web apps. ","fork":true,"url":"https://api.github.com/repos/collectiveidea/liquid","forks_url":"https://api.github.com/repos/collectiveidea/liquid/forks","keys_url":"https://api.github.com/repos/collectiveidea/liquid/keys{/key_id}","collaborators_url":"https://api.github.com/repos/collectiveidea/liquid/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/collectiveidea/liquid/teams","hooks_url":"https://api.github.com/repos/collectiveidea/liquid/hooks","issue_events_url":"https://api.github.com/repos/collectiveidea/liquid/issues/events{/number}","events_url":"https://api.github.com/repos/collectiveidea/liquid/events","assignees_url":"https://api.github.com/repos/collectiveidea/liquid/assignees{/user}","branches_url":"https://api.github.com/repos/collectiveidea/liquid/branches{/branch}","tags_url":"https://api.github.com/repos/collectiveidea/liquid/tags","blobs_url":"https://api.github.com/repos/collectiveidea/liquid/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/collectiveidea/liquid/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/collectiveidea/liquid/git/refs{/sha}","trees_url":"https://api.github.com/repos/collectiveidea/liquid/git/trees{/sha}","statuses_url":"https://api.github.com/repos/collectiveidea/liquid/statuses/{sha}","languages_url":"https://api.github.com/repos/collectiveidea/liquid/languages","stargazers_url":"https://api.github.com/repos/collectiveidea/liquid/stargazers","contributors_url":"https://api.github.com/repos/collectiveidea/liquid/contributors","subscribers_url":"https://api.github.com/repos/collectiveidea/liquid/subscribers","subscription_url":"https://api.github.com/repos/collectiveidea/liquid/subscription","commits_url":"https://api.github.com/repos/collectiveidea/liquid/commits{/sha}","git_commits_url":"https://api.github.com/repos/collectiveidea/liquid/git/commits{/sha}","comments_url":"https://api.github.com/repos/collectiveidea/liquid/comments{/number}","issue_comment_url":"https://api.github.com/repos/collectiveidea/liquid/issues/comments/{number}","contents_url":"https://api.github.com/repos/collectiveidea/liquid/contents/{+path}","compare_url":"https://api.github.com/repos/collectiveidea/liquid/compare/{base}...{head}","merges_url":"https://api.github.com/repos/collectiveidea/liquid/merges","archive_url":"https://api.github.com/repos/collectiveidea/liquid/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/collectiveidea/liquid/downloads","issues_url":"https://api.github.com/repos/collectiveidea/liquid/issues{/number}","pulls_url":"https://api.github.com/repos/collectiveidea/liquid/pulls{/number}","milestones_url":"https://api.github.com/repos/collectiveidea/liquid/milestones{/number}","notifications_url":"https://api.github.com/repos/collectiveidea/liquid/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/collectiveidea/liquid/labels{/name}","releases_url":"https://api.github.com/repos/collectiveidea/liquid/releases{/id}","created_at":"2014-05-23T20:16:42Z","updated_at":"2014-06-24T15:27:46Z","pushed_at":"2014-08-11T17:20:16Z","git_url":"git://github.com/collectiveidea/liquid.git","ssh_url":"git@github.com:collectiveidea/liquid.git","clone_url":"https://github.com/collectiveidea/liquid.git","svn_url":"https://github.com/collectiveidea/liquid","homepage":"http://liquidmarkup.org/","size":1447,"stargazers_count":1,"watchers_count":1,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"Shopify:master","ref":"master","sha":"79d4ec1a4871ecccdb780213735a61a1c47b257e","user":{"login":"Shopify","id":8085,"avatar_url":"https://avatars.githubusercontent.com/u/8085?v=2","gravatar_id":"c5c29e455b8c4f05112abbc01af58a29","url":"https://api.github.com/users/Shopify","html_url":"https://github.com/Shopify","followers_url":"https://api.github.com/users/Shopify/followers","following_url":"https://api.github.com/users/Shopify/following{/other_user}","gists_url":"https://api.github.com/users/Shopify/gists{/gist_id}","starred_url":"https://api.github.com/users/Shopify/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shopify/subscriptions","organizations_url":"https://api.github.com/users/Shopify/orgs","repos_url":"https://api.github.com/users/Shopify/repos","events_url":"https://api.github.com/users/Shopify/events{/privacy}","received_events_url":"https://api.github.com/users/Shopify/received_events","type":"Organization","site_admin":false},"repo":{"id":15435,"name":"liquid","full_name":"Shopify/liquid","owner":{"login":"Shopify","id":8085,"avatar_url":"https://avatars.githubusercontent.com/u/8085?v=2","gravatar_id":"c5c29e455b8c4f05112abbc01af58a29","url":"https://api.github.com/users/Shopify","html_url":"https://github.com/Shopify","followers_url":"https://api.github.com/users/Shopify/followers","following_url":"https://api.github.com/users/Shopify/following{/other_user}","gists_url":"https://api.github.com/users/Shopify/gists{/gist_id}","starred_url":"https://api.github.com/users/Shopify/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shopify/subscriptions","organizations_url":"https://api.github.com/users/Shopify/orgs","repos_url":"https://api.github.com/users/Shopify/repos","events_url":"https://api.github.com/users/Shopify/events{/privacy}","received_events_url":"https://api.github.com/users/Shopify/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Shopify/liquid","description":"Liquid markup language. Safe, customer facing template language for flexible web apps. ","fork":false,"url":"https://api.github.com/repos/Shopify/liquid","forks_url":"https://api.github.com/repos/Shopify/liquid/forks","keys_url":"https://api.github.com/repos/Shopify/liquid/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Shopify/liquid/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Shopify/liquid/teams","hooks_url":"https://api.github.com/repos/Shopify/liquid/hooks","issue_events_url":"https://api.github.com/repos/Shopify/liquid/issues/events{/number}","events_url":"https://api.github.com/repos/Shopify/liquid/events","assignees_url":"https://api.github.com/repos/Shopify/liquid/assignees{/user}","branches_url":"https://api.github.com/repos/Shopify/liquid/branches{/branch}","tags_url":"https://api.github.com/repos/Shopify/liquid/tags","blobs_url":"https://api.github.com/repos/Shopify/liquid/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Shopify/liquid/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Shopify/liquid/git/refs{/sha}","trees_url":"https://api.github.com/repos/Shopify/liquid/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Shopify/liquid/statuses/{sha}","languages_url":"https://api.github.com/repos/Shopify/liquid/languages","stargazers_url":"https://api.github.com/repos/Shopify/liquid/stargazers","contributors_url":"https://api.github.com/repos/Shopify/liquid/contributors","subscribers_url":"https://api.github.com/repos/Shopify/liquid/subscribers","subscription_url":"https://api.github.com/repos/Shopify/liquid/subscription","commits_url":"https://api.github.com/repos/Shopify/liquid/commits{/sha}","git_commits_url":"https://api.github.com/repos/Shopify/liquid/git/commits{/sha}","comments_url":"https://api.github.com/repos/Shopify/liquid/comments{/number}","issue_comment_url":"https://api.github.com/repos/Shopify/liquid/issues/comments/{number}","contents_url":"https://api.github.com/repos/Shopify/liquid/contents/{+path}","compare_url":"https://api.github.com/repos/Shopify/liquid/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Shopify/liquid/merges","archive_url":"https://api.github.com/repos/Shopify/liquid/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Shopify/liquid/downloads","issues_url":"https://api.github.com/repos/Shopify/liquid/issues{/number}","pulls_url":"https://api.github.com/repos/Shopify/liquid/pulls{/number}","milestones_url":"https://api.github.com/repos/Shopify/liquid/milestones{/number}","notifications_url":"https://api.github.com/repos/Shopify/liquid/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Shopify/liquid/labels{/name}","releases_url":"https://api.github.com/repos/Shopify/liquid/releases{/id}","created_at":"2008-05-08T15:27:26Z","updated_at":"2014-08-12T11:59:01Z","pushed_at":"2014-08-12T12:58:31Z","git_url":"git://github.com/Shopify/liquid.git","ssh_url":"git@github.com:Shopify/liquid.git","clone_url":"https://github.com/Shopify/liquid.git","svn_url":"https://github.com/Shopify/liquid","homepage":"http://liquidmarkup.org/","size":10429,"stargazers_count":2609,"watchers_count":2609,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":353,"mirror_url":null,"open_issues_count":15,"forks":353,"open_issues":15,"watchers":2609,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/364"},"html":{"href":"https://github.com/Shopify/liquid/pull/364"},"issue":{"href":"https://api.github.com/repos/Shopify/liquid/issues/364"},"comments":{"href":"https://api.github.com/repos/Shopify/liquid/issues/364/comments"},"review_comments":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/364/comments"},"review_comment":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/Shopify/liquid/pulls/364/commits"},"statuses":{"href":"https://api.github.com/repos/Shopify/liquid/statuses/dcc6ca1aa198e6df57da7aeb0c5f92e336fc481a"}}}}