GitHub PullRequestReviewCommentEvent at 2014-11-25 05:55:19
Repository: spree
Actor: houndci
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/spree/spree/pulls/comments/20844636","id":20844636,"diff_hunk":"@@ -93,18 +118,30 @@ module Stock\n           let(:shipping_methods) { 2.times.map { create(:shipping_method) } }\n \n           it \"selects the most affordable shipping rate\" do\n-            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(5.00)\n-            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(3.00)\n+            calculators = [\n+              Calculator.new(calculable_id: shipping_methods[0].id),\n+              Calculator.new(calculable_id: shipping_methods[1].id)\n+            ]\n+\n+            allow(calculators[0]).to receive(:compute).and_return(5.00)\n+            allow(calculators[1]).to receive(:compute).and_return(3.00)\n \n+            allow(subject).to receive_messages(:calculators => calculators)\n             allow(subject).to receive(:shipping_methods).and_return(shipping_methods)\n \n             expect(subject.shipping_rates(package).sort_by(&:cost).map(&:selected)).to eq [true, false]\n           end\n \n           it \"selects the most affordable shipping rate and doesn't raise exception over nil cost\" do\n-            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(1.00)\n-            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(nil)\n+            calculators = [\n+              Calculator.new(calculable_id: shipping_methods[0].id),\n+              Calculator.new(calculable_id: shipping_methods[1].id)\n+            ]\n \n+            allow(calculators[0]).to receive(:compute).and_return(1.00)\n+            allow(calculators[1]).to receive(:compute).and_return(nil)\n+\n+            allow(subject).to receive_messages(:calculators => calculators)","path":"core/spec/models/spree/stock/estimator_spec.rb","position":104,"original_position":104,"commit_id":"41bdf916a8356201d292c4fc5ab1e89e9704f24b","original_commit_id":"41bdf916a8356201d292c4fc5ab1e89e9704f24b","user":{"login":"houndci","id":6697940,"avatar_url":"https://avatars.githubusercontent.com/u/6697940?v=3","gravatar_id":"","url":"https://api.github.com/users/houndci","html_url":"https://github.com/houndci","followers_url":"https://api.github.com/users/houndci/followers","following_url":"https://api.github.com/users/houndci/following{/other_user}","gists_url":"https://api.github.com/users/houndci/gists{/gist_id}","starred_url":"https://api.github.com/users/houndci/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/houndci/subscriptions","organizations_url":"https://api.github.com/users/houndci/orgs","repos_url":"https://api.github.com/users/houndci/repos","events_url":"https://api.github.com/users/houndci/events{/privacy}","received_events_url":"https://api.github.com/users/houndci/received_events","type":"User","site_admin":false},"body":"Use the new Ruby 1.9 hash syntax.","created_at":"2014-11-25T05:55:19Z","updated_at":"2014-11-25T05:55:19Z","html_url":"https://github.com/spree/spree/pull/5676#discussion_r20844636","pull_request_url":"https://api.github.com/repos/spree/spree/pulls/5676","_links":{"self":{"href":"https://api.github.com/repos/spree/spree/pulls/comments/20844636"},"html":{"href":"https://github.com/spree/spree/pull/5676#discussion_r20844636"},"pull_request":{"href":"https://api.github.com/repos/spree/spree/pulls/5676"}}},"pull_request":{"url":"https://api.github.com/repos/spree/spree/pulls/5676","id":24986498,"html_url":"https://github.com/spree/spree/pull/5676","diff_url":"https://github.com/spree/spree/pull/5676.diff","patch_url":"https://github.com/spree/spree/pull/5676.patch","issue_url":"https://api.github.com/repos/spree/spree/issues/5676","number":5676,"state":"open","locked":false,"title":"Using more objects in memory during checkout","user":{"login":"huoxito","id":245662,"avatar_url":"https://avatars.githubusercontent.com/u/245662?v=3","gravatar_id":"","url":"https://api.github.com/users/huoxito","html_url":"https://github.com/huoxito","followers_url":"https://api.github.com/users/huoxito/followers","following_url":"https://api.github.com/users/huoxito/following{/other_user}","gists_url":"https://api.github.com/users/huoxito/gists{/gist_id}","starred_url":"https://api.github.com/users/huoxito/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/huoxito/subscriptions","organizations_url":"https://api.github.com/users/huoxito/orgs","repos_url":"https://api.github.com/users/huoxito/repos","events_url":"https://api.github.com/users/huoxito/events{/privacy}","received_events_url":"https://api.github.com/users/huoxito/received_events","type":"User","site_admin":false},"body":"I've been trying to address some points we always consider when thinking of how to resolve / improve spree performance. Working with objects in memory, doing less `object.reload` and saying good bye to famous `order.update!`.\r\n\r\nI started by trying to reduce db queries / ar object loads when adding items to cart (`order.contents.add`), could go as far as running about half less db queries, see https://gist.github.com/huoxito/3d8bcf0d8fa24f54f429, and I've been trying to somehow benchmark this but I'm not really sure how to do it best yet.\r\n\r\nThese are some from a simple `Benchmark.measure` script https://gist.github.com/huoxito/2e0bc0b97420eb032329. The difference is not much at all really from a simple default spree install (with sample data only). But I'm curious to see how this would go with some more real world store scenario.\r\n\r\nAlso started looking at the transition from address => delivery and made some changes on the Stock::Estimator to reduce the number of hits on db as well.\r\n\r\nI couldn't make all specs pass yet mostly because I'd need more time to catch where is it exactly we're missing fresh data and using the out of date objects from the database (there are some NOTEs on the code though that should help narrow down the hot spots). \r\n\r\nOverall I feel like the end result here is going to be more code and possibly more complex code but I can't really see how else we can achieve better performance. Personally every time I've done something like `order.reload` is because I had lost track of the method calls and just reloading the object, or everything inside spree, would be the easiest way to go.\r\n\r\nwill still try to make those specs green but I wanted to share this with everyone to get feedback since I've spent already quite a decent amount of time on it.\r\n\r\n// @BDQ @JDutil ","created_at":"2014-11-25T05:55:08Z","updated_at":"2014-11-25T05:55:20Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b7c915f85c9790e93c4f62f29ac18609ebfc4345","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/spree/spree/pulls/5676/commits","review_comments_url":"https://api.github.com/repos/spree/spree/pulls/5676/comments","review_comment_url":"https://api.github.com/repos/spree/spree/pulls/comments/{number}","comments_url":"https://api.github.com/repos/spree/spree/issues/5676/comments","statuses_url":"https://api.github.com/repos/spree/spree/statuses/41bdf916a8356201d292c4fc5ab1e89e9704f24b","head":{"label":"huoxito:checkout-queries-review","ref":"checkout-queries-review","sha":"41bdf916a8356201d292c4fc5ab1e89e9704f24b","user":{"login":"huoxito","id":245662,"avatar_url":"https://avatars.githubusercontent.com/u/245662?v=3","gravatar_id":"","url":"https://api.github.com/users/huoxito","html_url":"https://github.com/huoxito","followers_url":"https://api.github.com/users/huoxito/followers","following_url":"https://api.github.com/users/huoxito/following{/other_user}","gists_url":"https://api.github.com/users/huoxito/gists{/gist_id}","starred_url":"https://api.github.com/users/huoxito/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/huoxito/subscriptions","organizations_url":"https://api.github.com/users/huoxito/orgs","repos_url":"https://api.github.com/users/huoxito/repos","events_url":"https://api.github.com/users/huoxito/events{/privacy}","received_events_url":"https://api.github.com/users/huoxito/received_events","type":"User","site_admin":false},"repo":{"id":6618582,"name":"spree","full_name":"huoxito/spree","owner":{"login":"huoxito","id":245662,"avatar_url":"https://avatars.githubusercontent.com/u/245662?v=3","gravatar_id":"","url":"https://api.github.com/users/huoxito","html_url":"https://github.com/huoxito","followers_url":"https://api.github.com/users/huoxito/followers","following_url":"https://api.github.com/users/huoxito/following{/other_user}","gists_url":"https://api.github.com/users/huoxito/gists{/gist_id}","starred_url":"https://api.github.com/users/huoxito/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/huoxito/subscriptions","organizations_url":"https://api.github.com/users/huoxito/orgs","repos_url":"https://api.github.com/users/huoxito/repos","events_url":"https://api.github.com/users/huoxito/events{/privacy}","received_events_url":"https://api.github.com/users/huoxito/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/huoxito/spree","description":"Spree is a complete open source e-commerce solution for Ruby on Rails.","fork":true,"url":"https://api.github.com/repos/huoxito/spree","forks_url":"https://api.github.com/repos/huoxito/spree/forks","keys_url":"https://api.github.com/repos/huoxito/spree/keys{/key_id}","collaborators_url":"https://api.github.com/repos/huoxito/spree/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/huoxito/spree/teams","hooks_url":"https://api.github.com/repos/huoxito/spree/hooks","issue_events_url":"https://api.github.com/repos/huoxito/spree/issues/events{/number}","events_url":"https://api.github.com/repos/huoxito/spree/events","assignees_url":"https://api.github.com/repos/huoxito/spree/assignees{/user}","branches_url":"https://api.github.com/repos/huoxito/spree/branches{/branch}","tags_url":"https://api.github.com/repos/huoxito/spree/tags","blobs_url":"https://api.github.com/repos/huoxito/spree/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/huoxito/spree/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/huoxito/spree/git/refs{/sha}","trees_url":"https://api.github.com/repos/huoxito/spree/git/trees{/sha}","statuses_url":"https://api.github.com/repos/huoxito/spree/statuses/{sha}","languages_url":"https://api.github.com/repos/huoxito/spree/languages","stargazers_url":"https://api.github.com/repos/huoxito/spree/stargazers","contributors_url":"https://api.github.com/repos/huoxito/spree/contributors","subscribers_url":"https://api.github.com/repos/huoxito/spree/subscribers","subscription_url":"https://api.github.com/repos/huoxito/spree/subscription","commits_url":"https://api.github.com/repos/huoxito/spree/commits{/sha}","git_commits_url":"https://api.github.com/repos/huoxito/spree/git/commits{/sha}","comments_url":"https://api.github.com/repos/huoxito/spree/comments{/number}","issue_comment_url":"https://api.github.com/repos/huoxito/spree/issues/comments/{number}","contents_url":"https://api.github.com/repos/huoxito/spree/contents/{+path}","compare_url":"https://api.github.com/repos/huoxito/spree/compare/{base}...{head}","merges_url":"https://api.github.com/repos/huoxito/spree/merges","archive_url":"https://api.github.com/repos/huoxito/spree/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/huoxito/spree/downloads","issues_url":"https://api.github.com/repos/huoxito/spree/issues{/number}","pulls_url":"https://api.github.com/repos/huoxito/spree/pulls{/number}","milestones_url":"https://api.github.com/repos/huoxito/spree/milestones{/number}","notifications_url":"https://api.github.com/repos/huoxito/spree/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/huoxito/spree/labels{/name}","releases_url":"https://api.github.com/repos/huoxito/spree/releases{/id}","created_at":"2012-11-09T19:17:27Z","updated_at":"2014-06-23T02:21:27Z","pushed_at":"2014-11-25T05:21:33Z","git_url":"git://github.com/huoxito/spree.git","ssh_url":"git@github.com:huoxito/spree.git","clone_url":"https://github.com/huoxito/spree.git","svn_url":"https://github.com/huoxito/spree","homepage":"http://spreecommerce.com","size":73842,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"spree:master","ref":"master","sha":"d5c4ed35e0a65d98a8243dd5b641ee6f2c2e4073","user":{"login":"spree","id":56702,"avatar_url":"https://avatars.githubusercontent.com/u/56702?v=3","gravatar_id":"","url":"https://api.github.com/users/spree","html_url":"https://github.com/spree","followers_url":"https://api.github.com/users/spree/followers","following_url":"https://api.github.com/users/spree/following{/other_user}","gists_url":"https://api.github.com/users/spree/gists{/gist_id}","starred_url":"https://api.github.com/users/spree/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spree/subscriptions","organizations_url":"https://api.github.com/users/spree/orgs","repos_url":"https://api.github.com/users/spree/repos","events_url":"https://api.github.com/users/spree/events{/privacy}","received_events_url":"https://api.github.com/users/spree/received_events","type":"Organization","site_admin":false},"repo":{"id":3314,"name":"spree","full_name":"spree/spree","owner":{"login":"spree","id":56702,"avatar_url":"https://avatars.githubusercontent.com/u/56702?v=3","gravatar_id":"","url":"https://api.github.com/users/spree","html_url":"https://github.com/spree","followers_url":"https://api.github.com/users/spree/followers","following_url":"https://api.github.com/users/spree/following{/other_user}","gists_url":"https://api.github.com/users/spree/gists{/gist_id}","starred_url":"https://api.github.com/users/spree/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spree/subscriptions","organizations_url":"https://api.github.com/users/spree/orgs","repos_url":"https://api.github.com/users/spree/repos","events_url":"https://api.github.com/users/spree/events{/privacy}","received_events_url":"https://api.github.com/users/spree/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/spree/spree","description":"Spree is a complete open source e-commerce solution for Ruby on Rails.","fork":false,"url":"https://api.github.com/repos/spree/spree","forks_url":"https://api.github.com/repos/spree/spree/forks","keys_url":"https://api.github.com/repos/spree/spree/keys{/key_id}","collaborators_url":"https://api.github.com/repos/spree/spree/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/spree/spree/teams","hooks_url":"https://api.github.com/repos/spree/spree/hooks","issue_events_url":"https://api.github.com/repos/spree/spree/issues/events{/number}","events_url":"https://api.github.com/repos/spree/spree/events","assignees_url":"https://api.github.com/repos/spree/spree/assignees{/user}","branches_url":"https://api.github.com/repos/spree/spree/branches{/branch}","tags_url":"https://api.github.com/repos/spree/spree/tags","blobs_url":"https://api.github.com/repos/spree/spree/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/spree/spree/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/spree/spree/git/refs{/sha}","trees_url":"https://api.github.com/repos/spree/spree/git/trees{/sha}","statuses_url":"https://api.github.com/repos/spree/spree/statuses/{sha}","languages_url":"https://api.github.com/repos/spree/spree/languages","stargazers_url":"https://api.github.com/repos/spree/spree/stargazers","contributors_url":"https://api.github.com/repos/spree/spree/contributors","subscribers_url":"https://api.github.com/repos/spree/spree/subscribers","subscription_url":"https://api.github.com/repos/spree/spree/subscription","commits_url":"https://api.github.com/repos/spree/spree/commits{/sha}","git_commits_url":"https://api.github.com/repos/spree/spree/git/commits{/sha}","comments_url":"https://api.github.com/repos/spree/spree/comments{/number}","issue_comment_url":"https://api.github.com/repos/spree/spree/issues/comments/{number}","contents_url":"https://api.github.com/repos/spree/spree/contents/{+path}","compare_url":"https://api.github.com/repos/spree/spree/compare/{base}...{head}","merges_url":"https://api.github.com/repos/spree/spree/merges","archive_url":"https://api.github.com/repos/spree/spree/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/spree/spree/downloads","issues_url":"https://api.github.com/repos/spree/spree/issues{/number}","pulls_url":"https://api.github.com/repos/spree/spree/pulls{/number}","milestones_url":"https://api.github.com/repos/spree/spree/milestones{/number}","notifications_url":"https://api.github.com/repos/spree/spree/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/spree/spree/labels{/name}","releases_url":"https://api.github.com/repos/spree/spree/releases{/id}","created_at":"2008-03-10T14:45:35Z","updated_at":"2014-11-25T05:50:18Z","pushed_at":"2014-11-25T02:39:07Z","git_url":"git://github.com/spree/spree.git","ssh_url":"git@github.com:spree/spree.git","clone_url":"https://github.com/spree/spree.git","svn_url":"https://github.com/spree/spree","homepage":"http://spreecommerce.com","size":139866,"stargazers_count":5685,"watchers_count":5685,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2638,"mirror_url":null,"open_issues_count":113,"forks":2638,"open_issues":113,"watchers":5685,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/spree/spree/pulls/5676"},"html":{"href":"https://github.com/spree/spree/pull/5676"},"issue":{"href":"https://api.github.com/repos/spree/spree/issues/5676"},"comments":{"href":"https://api.github.com/repos/spree/spree/issues/5676/comments"},"review_comments":{"href":"https://api.github.com/repos/spree/spree/pulls/5676/comments"},"review_comment":{"href":"https://api.github.com/repos/spree/spree/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/spree/spree/pulls/5676/commits"},"statuses":{"href":"https://api.github.com/repos/spree/spree/statuses/41bdf916a8356201d292c4fc5ab1e89e9704f24b"}}}}