diff --git a/lib/git-review/commands.rb b/lib/git-review/commands.rb index 6d3d0c5..17806db 100644 --- a/lib/git-review/commands.rb +++ b/lib/git-review/commands.rb @@ -12,14 +12,13 @@ def list(reverse=false) # explicitly look for local changes Github does not yet know about local.merged?(request.head.sha) } - requests.reverse! if reverse source = local.source if requests.empty? puts "No pending requests for '#{source}'." else puts "Pending requests for '#{source}':" puts "ID Updated Comments Title" - requests.each { |request| print_request(request) } + print_requests(requests, reverse) end end @@ -173,18 +172,31 @@ def console private - def print_request(request) + def request_summary_line(request) date_string = format_time(request.updated_at) - comments_count = request.comments.to_i + request.review_comments.to_i + comments_count = github.comments_count(request) line = format_text(request.number, 8) line << format_text(date_string, 11) line << format_text(comments_count, 10) line << format_text(request.title, 91) - puts line + line + end + + def print_requests(requests, reverse=false) + # put all output lines in a hash first, keyed by request number + # this is to make sure the order is still correct even if we use + # multi-threading to retrieve the requests + output = {} + requests.each { |req| output[req.number] = request_summary_line(req) } + numbers = output.keys.sort + numbers.reverse! if reverse + numbers.each do |n| + puts output[n] + end end def print_request_details(request) - comments_count = request.comments.to_i + request.review_comments.to_i + comments_count = github.comments_count(request) puts 'ID : ' + request.number.to_s puts 'Label : ' + request.head.label puts 'Updated : ' + format_time(request.updated_at) diff --git a/lib/git-review/github.rb b/lib/git-review/github.rb index 0b0b69a..334be57 100644 --- a/lib/git-review/github.rb +++ b/lib/git-review/github.rb @@ -71,9 +71,15 @@ def current_requests(repo=source_repo) # a more detailed collection of requests def current_requests_full(repo=source_repo) - @github.pull_requests(repo).collect { |request| - @github.pull_request(repo, request.number) - } + threads = [] + requests = [] + @github.pull_requests(repo).each do |req| + threads << Thread.new { + requests << @github.pull_request(repo, req.number) + } + end + threads.each { |t| t.join } + requests end def update @@ -144,7 +150,8 @@ def commit_discussion(number) end def issue_discussion(number) - comments = @github.issue_comments(source_repo, number) + comments = @github.issue_comments(source_repo, number) + + @github.review_comments(source_repo, number) discussion = ["\nComments on pull request:\n\n"] discussion += comments.collect { |comment| name = comment.user.login @@ -161,6 +168,14 @@ def issue_discussion(number) discussion.compact.flatten unless discussion.empty? end + # get the number of comments, including comments on commits + def comments_count(request) + issue_c = request.comments + request.review_comments + commits_c = @github.pull_commits(source_repo, request.number). + inject(0) { |sum, c| sum + c.commit.comment_count } + issue_c + commits_c + end + # show discussion for a request def discussion(number) commit_discussion(number) + diff --git a/spec/git-review/commands_spec.rb b/spec/git-review/commands_spec.rb index a64ba61..6e5cf45 100644 --- a/spec/git-review/commands_spec.rb +++ b/spec/git-review/commands_spec.rb @@ -33,15 +33,13 @@ it 'shows them' do subject.should_receive(:puts).with(/Pending requests for 'some_source'/) subject.should_not_receive(:puts).with(/No pending requests/) - subject.should_receive(:print_request).with(req1).ordered - subject.should_receive(:print_request).with(req2).ordered + subject.should_receive(:print_requests).with([req1, req2], false) subject.list end it 'sorts the output with --reverse option' do subject.stub(:puts) - subject.should_receive(:print_request).with(req2).ordered - subject.should_receive(:print_request).with(req1).ordered + subject.should_receive(:print_requests).with([req1, req2], true) subject.list(true) end