From c2db6dafe5b35c2a6c8a016bf047887968e28b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Wed, 15 Oct 2014 15:09:06 +0300 Subject: [PATCH] Refactor Github to receive repo via constructor Simplifies the whole class and moves the slug parsing to a more natural place. --- lib/pronto.rb | 1 - lib/pronto/formatter/github_formatter.rb | 13 ++--- .../github_pull_request_formatter.rb | 13 ++--- lib/pronto/git/remote.rb | 12 ----- lib/pronto/git/repository.rb | 12 ++--- lib/pronto/github.rb | 52 +++++++++++-------- spec/pronto/git/remote_spec.rb | 23 -------- spec/pronto/github_spec.rb | 30 +++++++++-- 8 files changed, 71 insertions(+), 85 deletions(-) delete mode 100644 lib/pronto/git/remote.rb delete mode 100644 spec/pronto/git/remote_spec.rb diff --git a/lib/pronto.rb b/lib/pronto.rb index 9d7524ab..537cba09 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -6,7 +6,6 @@ require 'pronto/git/patches' require 'pronto/git/patch' require 'pronto/git/line' -require 'pronto/git/remote' require 'pronto/plugin' require 'pronto/message' diff --git a/lib/pronto/formatter/github_formatter.rb b/lib/pronto/formatter/github_formatter.rb index 9c0092e2..fb76936c 100644 --- a/lib/pronto/formatter/github_formatter.rb +++ b/lib/pronto/formatter/github_formatter.rb @@ -3,6 +3,7 @@ module Formatter class GithubFormatter def format(messages, repo) messages = messages.uniq { |message| [message.msg, message.line.new_lineno] } + client = Github.new(repo) commit_messages = messages.map do |message| sha = message.commit_sha @@ -10,7 +11,7 @@ def format(messages, repo) path = message.path position = message.line.commit_line.position if message.line - create_comment(repo, sha, body, path, position) + create_comment(client, sha, body, path, position) end "#{commit_messages.compact.count} Pronto messages posted to GitHub" @@ -18,16 +19,12 @@ def format(messages, repo) private - def create_comment(repo, sha, body, path, position) - comment = Github::Comment.new(repo, sha, body, path, position) - comments = client.commit_comments(repo, sha) + def create_comment(client, sha, body, path, position) + comment = Github::Comment.new(sha, body, path, position) + comments = client.commit_comments(sha) existing = comments.any? { |c| comment == c } client.create_commit_comment(comment) unless existing end - - def client - @client ||= Github.new - end end end end diff --git a/lib/pronto/formatter/github_pull_request_formatter.rb b/lib/pronto/formatter/github_pull_request_formatter.rb index db9223c3..bea28169 100644 --- a/lib/pronto/formatter/github_pull_request_formatter.rb +++ b/lib/pronto/formatter/github_pull_request_formatter.rb @@ -3,6 +3,7 @@ module Formatter class GithubPullRequestFormatter def format(messages, repo) messages = messages.uniq { |message| [message.msg, message.line.new_lineno] } + client = Github.new(repo) commit_messages = messages.map do |message| body = message.msg @@ -17,7 +18,7 @@ def format(messages, repo) line end - create_comment(repo, sha, body, path, line.position) + create_comment(client, sha, body, path, line.position) end "#{commit_messages.compact.count} Pronto messages posted to GitHub" @@ -25,16 +26,12 @@ def format(messages, repo) private - def create_comment(repo, sha, body, path, position) - comment = Github::Comment.new(repo, sha, body, path, position) - comments = client.pull_comments(repo, sha) + def create_comment(client, sha, body, path, position) + comment = Github::Comment.new(sha, body, path, position) + comments = client.pull_comments(sha) existing = comments.any? { |c| comment == c } client.create_pull_comment(comment) unless existing end - - def client - @client ||= Github.new - end end end end diff --git a/lib/pronto/git/remote.rb b/lib/pronto/git/remote.rb deleted file mode 100644 index 8797b9d7..00000000 --- a/lib/pronto/git/remote.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Pronto - module Git - class Remote < Struct.new(:remote) - def github_slug - @github_slug ||= begin - match = /.*github.com(:|\/)(?.*).git/.match(remote.url) - match[:slug] if match - end - end - end - end -end diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb index 3e58762f..1bdc5666 100644 --- a/lib/pronto/git/repository.rb +++ b/lib/pronto/git/repository.rb @@ -7,10 +7,6 @@ def initialize(path) @repo = Rugged::Repository.new(path) end - def github_slug - remotes.map(&:github_slug).compact.first - end - def diff(commit) if commit == :index patches = @repo.index.diff @@ -59,6 +55,10 @@ def branch @repo.head.name.sub('refs/heads/', '') if @repo.head.branch? end + def remote_urls + @repo.remotes.map(&:url) + end + private def empty_patches(sha) @@ -72,10 +72,6 @@ def merge_base(commit) def head @repo.head.target end - - def remotes - @remotes ||= @repo.remotes.map { |remote| Remote.new(remote) } - end end end end diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb index 75ccfd57..5bab129b 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -1,55 +1,63 @@ module Pronto class Github - def initialize + def initialize(repo) + @repo = repo @comment_cache = {} @pull_id_cache = {} end - def pull_comments(repo, sha) - pull_id = pull_id(repo) - @comment_cache["#{repo.github_slug}/#{pull_id}/#{sha}"] ||= begin - client.pull_comments(repo.github_slug, pull_id).map do |comment| - Comment.new(repo, sha, comment.body, comment.path, comment.position) + def pull_comments(sha) + @comment_cache["#{pull_id}/#{sha}"] ||= begin + client.pull_comments(slug, pull_id).map do |comment| + Comment.new(sha, comment.body, comment.path, comment.position) end end end - def commit_comments(repo, sha) - @comment_cache["#{repo.github_slug}/#{sha}"] ||= begin - client.commit_comments(repo.github_slug, sha).map do |comment| - Comment.new(repo, sha, comment.body, comment.path, comment.position) + def commit_comments(sha) + @comment_cache["#{sha}"] ||= begin + client.commit_comments(slug, sha).map do |comment| + Comment.new(sha, comment.body, comment.path, comment.position) end end end def create_commit_comment(comment) - client.create_commit_comment(comment.repo.github_slug, comment.sha, - comment.body, comment.path, nil, comment.position) + client.create_commit_comment(slug, comment.sha, comment.body, + comment.path, nil, comment.position) end def create_pull_comment(comment) - pull_id = pull_id(comment.repo) - client.create_pull_comment(comment.repo.github_slug, pull_id, - comment.body, comment.sha, comment.path, comment.position) + client.create_pull_comment(slug, pull_id, comment.body, + comment.sha, comment.path, comment.position) end private + def slug + @slug ||= begin + @repo.remote_urls.map do |url| + match = /.*github.com(:|\/)(?.*).git/.match(url) + match[:slug] if match + end.compact.first + end + end + def client @client ||= Octokit::Client.new(access_token: access_token) end - def pull_requests(repo) - client.pull_requests(repo.github_slug) + def pull_requests + @pull_requests ||= client.pull_requests(slug) end - def pull_id(repo) - @pull_id_cache["#{repo.github_slug}"] ||= begin + def pull_id + @pull_id ||= begin pull_id = ENV['PULL_REQUEST_ID'] if pull_id pull_id.to_i - elsif repo.branch - pull = pull_requests(repo).find { |pr| pr[:head][:ref] == repo.branch } + elsif @repo.branch + pull = pull_requests.find { |pr| pr[:head][:ref] == @repo.branch } pull[:number].to_i if pull end end @@ -59,7 +67,7 @@ def access_token ENV['GITHUB_ACCESS_TOKEN'] end - class Comment < Struct.new(:repo, :sha, :body, :path, :position) + class Comment < Struct.new(:sha, :body, :path, :position) def ==(other) position == other.position && path == other.path && diff --git a/spec/pronto/git/remote_spec.rb b/spec/pronto/git/remote_spec.rb deleted file mode 100644 index e2452df4..00000000 --- a/spec/pronto/git/remote_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -module Pronto - module Git - describe Remote do - let(:remote) { Remote.new(double(url: url)) } - - describe '#github_slug' do - subject { remote.github_slug } - - context 'ssh' do - let(:url) { 'git@github.com:mmozuras/pronto.git' } - it { should == 'mmozuras/pronto' } - end - - context 'http' do - let(:url) { 'https://github.com/mmozuras/pronto.git' } - it { should == 'mmozuras/pronto' } - end - end - end - end -end diff --git a/spec/pronto/github_spec.rb b/spec/pronto/github_spec.rb index be2816e7..113f2de9 100644 --- a/spec/pronto/github_spec.rb +++ b/spec/pronto/github_spec.rb @@ -2,18 +2,19 @@ module Pronto describe Github do - let(:github) { Github.new } + let(:github) { Github.new(repo) } describe '#commit_comments' do - subject { github.commit_comments(repo, sha) } + subject { github.commit_comments(sha) } context 'three requests for same comments' do - let(:repo) { double(github_slug: 'mmozuras/pronto') } + let(:repo) { double(remote_urls: ['git@github.com:mmozuras/pronto.git']) } let(:sha) { '61e4bef' } specify do Octokit::Client.any_instance .should_receive(:commit_comments) + .with('mmozuras/pronto', sha) .once .and_return([]) @@ -23,5 +24,28 @@ module Pronto end end end + + describe '#pull_comments' do + subject { github.pull_comments(sha) } + + context 'three requests for same comments' do + let(:repo) { double(remote_urls: ['https://github.com/mmozuras/pronto.git']) } + let(:sha) { '61e4bef' } + + specify do + Octokit::Client.any_instance + .should_receive(:pull_comments) + .with('mmozuras/pronto', 10) + .once + .and_return([]) + + ENV['PULL_REQUEST_ID'] = '10' + + subject + subject + subject + end + end + end end end