When I run the review command, and a pull request already exists, I will display information about the pull #29

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@codenamev
Member

When I run the review command, and a pull request already exists, I will display information about the pull

https://www.pivotaltracker.com/story/show/35174723

pull request info

@nhance
Member
nhance commented Sep 12, 2012

LGTM

@spartan-developer spartan-developer commented on an outdated diff Sep 13, 2012
lib/git_reflow.rb
@@ -231,6 +219,40 @@ def comment_authors_for_pull_request(pull_request, options = {})
comment_authors -= [github_user]
end
+ def display_pull_request_summary(pull_request)
+ summary_data = {
+ "branches" => "#{pull_request.head.label} -> #{pull_request.base.label}",
+ "number" => pull_request.number,
+ "url" => pull_request.html_url,
+ "reviewed by" => (comment_authors_for_pull_request(pull_request).join(", ") || "nobody")
+ }
+
+ notices = ""
+
+ # check for needed lgtm's
+ pull_comments = pull_request_comments(pull_request)
+ if pull_comments.any?
@spartan-developer
spartan-developer Sep 13, 2012

should only check for comments by someone other than the pull requestor (?)

@spartan-developer spartan-developer commented on an outdated diff Sep 13, 2012
lib/git_reflow.rb
+ # check for needed lgtm's
+ pull_comments = pull_request_comments(pull_request)
+ if pull_comments.any?
+ open_comment_authors = find_authors_of_open_pull_request_comments(pull_request)
+ last_committed_at = Time.parse pull_request.head.repo.updated_at
+ lgtm_authors = comment_authors_for_pull_request(pull_request, :with => LGTM, :after => last_committed_at)
+
+ summary_data.merge!("Last comment" => pull_comments.last.body)
+ summary_data.merge!("LGTM given by" => "#{lgtm_authors.join(', ')}") if lgtm_authors.any?
+
+ notices << "[notice] You still need a LGTM from: #{open_comment_authors.join(', ')}\n" if open_comment_authors.any?
+ else
+ notices << "[notice] No one has reviewed your pull request...\n"
+ end
+
+ padding_size = summary_data.keys.max{|a,b| a.size <=> b.size }.size + 2
@spartan-developer
spartan-developer Sep 13, 2012
summary_data.keys.max_by(&:size) + 2

?

@codenamev codenamev commented on the diff Sep 14, 2012
git_reflow.gemspec
@@ -23,10 +23,10 @@ spec = Gem::Specification.new do |s|
s.add_development_dependency('aruba', '~> 0.4.6')
s.add_development_dependency('jeweler')
s.add_development_dependency('webmock')
- s.add_dependency('gli', '2.0.0')
- s.add_dependency('json_pure', '1.7.5')
@codenamev
codenamev Sep 14, 2012 Member

github_api gem adds this for us via multi_json gem dependency

@nhance nhance commented on the diff Sep 14, 2012
lib/git_reflow.rb
require 'open-uri'
require "highline/import"
require 'httpclient'
require 'github_api'
+require 'json/pure'
+require 'colorize'
@nhance
nhance Sep 14, 2012 Member

We should be careful adding dependencies.. every dependency we add decreases the number of situations where we can run this (i think)

I know I have issues with that faraday gem

@nhance
Member
nhance commented Sep 14, 2012

lgtm

@codenamev codenamev added a commit that closed this pull request Sep 17, 2012
@codenamev codenamev Colorized output and fixed issue with lgtm lookup
[Delivers #35174723]
Closes #29

LGTM given by: @nhance

Squashed commit of the following:

commit 26b5a64
Author: Valentino <valentino@reenhanced.com>
Date:   Fri Sep 14 11:06:17 2012 -0400

    Colorized output and fixed issue with lgtm lookup

commit 7497a85
Author: Valentino <valentino@reenhanced.com>
Date:   Wed Sep 12 10:35:03 2012 -0400

    When I run the review command, and a pull request already exists, I will display information about the pull
490f8e1
@codenamev codenamev closed this in 490f8e1 Sep 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment