From 3014746d59c640dfb80a3516642372a0825aa1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 17:24:39 +0300 Subject: [PATCH 01/30] Remove 'grit' from gemspec and add 'rugged' --- pronto.gemspec | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pronto.gemspec b/pronto.gemspec index 4bb2e798..99e46b18 100644 --- a/pronto.gemspec +++ b/pronto.gemspec @@ -26,10 +26,9 @@ Gem::Specification.new do |s| s.require_paths = ['lib'] s.executables << 'pronto' - s.add_runtime_dependency 'rugged', '>= 0.19.0', '<= 0.20.0' + s.add_runtime_dependency 'rugged', '~> 0.21', '>= 0.21.0' s.add_runtime_dependency 'thor', '~> 0.19', '>= 0.19.1' s.add_runtime_dependency 'octokit', '~> 2.7', '>= 2.7.1' - s.add_runtime_dependency 'grit', '~> 2.5', '>= 2.5.0' s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.0' s.add_development_dependency 'rspec', '~> 2.14', '>= 2.14.0' end From b5df6699c1e6ed573f0aaaa39c4e18a8711cf4e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 17:25:30 +0300 Subject: [PATCH 02/30] Move Rugged::Patch out of Diff namespace --- lib/pronto/rugged/diff/patch.rb | 21 --------------------- lib/pronto/rugged/patch.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 21 deletions(-) delete mode 100644 lib/pronto/rugged/diff/patch.rb create mode 100644 lib/pronto/rugged/patch.rb diff --git a/lib/pronto/rugged/diff/patch.rb b/lib/pronto/rugged/diff/patch.rb deleted file mode 100644 index efb30b3f..00000000 --- a/lib/pronto/rugged/diff/patch.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Rugged - class Diff - class Patch - def added_lines - lines.select(&:addition?) - end - - def deleted_lines - lines.select(&:deletion?) - end - - def new_file_full_path - delta.new_file_full_path - end - - def lines - map(&:lines).flatten.compact - end - end - end -end diff --git a/lib/pronto/rugged/patch.rb b/lib/pronto/rugged/patch.rb new file mode 100644 index 00000000..3700ec9f --- /dev/null +++ b/lib/pronto/rugged/patch.rb @@ -0,0 +1,27 @@ +module Rugged + class Patch + def additions + stat[0] + end + + def deletions + stat[1] + end + + def added_lines + lines.select(&:addition?) + end + + def deleted_lines + lines.select(&:deletion?) + end + + def new_file_full_path + delta.new_file_full_path + end + + def lines + map(&:lines).flatten.compact + end + end +end From d04a0ef56b89ee27d896cc3ad3679e4b9bc92b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 17:26:59 +0300 Subject: [PATCH 03/30] Remove patches that are not needed with rugged '0.2.1' --- lib/pronto.rb | 5 +---- lib/pronto/rugged/diff.rb | 6 ------ lib/pronto/rugged/repository.rb | 11 ----------- lib/pronto/rugged/tree.rb | 6 ------ 4 files changed, 1 insertion(+), 27 deletions(-) delete mode 100644 lib/pronto/rugged/diff.rb delete mode 100644 lib/pronto/rugged/repository.rb delete mode 100644 lib/pronto/rugged/tree.rb diff --git a/lib/pronto.rb b/lib/pronto.rb index 1dbb18dc..c38ddefe 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -1,11 +1,8 @@ require 'rugged' -require 'pronto/rugged/diff' +require 'pronto/rugged/patch' require 'pronto/rugged/diff/delta' -require 'pronto/rugged/diff/patch' require 'pronto/rugged/diff/line' -require 'pronto/rugged/tree' require 'pronto/rugged/remote' -require 'pronto/rugged/repository' require 'pronto/rugged/commit' require 'pronto/plugin' diff --git a/lib/pronto/rugged/diff.rb b/lib/pronto/rugged/diff.rb deleted file mode 100644 index 1b9dcd97..00000000 --- a/lib/pronto/rugged/diff.rb +++ /dev/null @@ -1,6 +0,0 @@ -module Rugged - class Diff - attr_reader :owner - alias_method :tree, :owner - end -end diff --git a/lib/pronto/rugged/repository.rb b/lib/pronto/rugged/repository.rb deleted file mode 100644 index 35e868d5..00000000 --- a/lib/pronto/rugged/repository.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'grit' - -module Rugged - class Repository - def blame(filepath) - # TODO: Using grit blame implementation for now. - # Replace it with Rugged implementation when it's available. - ::Grit::Repo.new(path).blame(filepath, 'HEAD') - end - end -end diff --git a/lib/pronto/rugged/tree.rb b/lib/pronto/rugged/tree.rb deleted file mode 100644 index 859d8390..00000000 --- a/lib/pronto/rugged/tree.rb +++ /dev/null @@ -1,6 +0,0 @@ -module Rugged - class Tree - attr_reader :owner - alias_method :repo, :owner - end -end From 65ce672b3073fd391f83cb1a6c716d4493cbce3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 21:22:21 +0300 Subject: [PATCH 04/30] Passing parents.first is no longer necessary for diff --- lib/pronto/rugged/commit.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pronto/rugged/commit.rb b/lib/pronto/rugged/commit.rb index 0782b016..60f14bcc 100644 --- a/lib/pronto/rugged/commit.rb +++ b/lib/pronto/rugged/commit.rb @@ -2,8 +2,7 @@ module Rugged class Commit def show # TODO: Rugged does not seem to support diffing against multiple parents - return if parents.count != 1 - diff(parents.first, reverse: true) + diff(reverse: true) unless parents.count != 1 end end end From e013c9b223c52ee2d8aa33e0fa6352cb6b60c9a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 21:44:46 +0300 Subject: [PATCH 05/30] Add 'pry' as development dependency --- pronto.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/pronto.gemspec b/pronto.gemspec index 99e46b18..01fb5cd4 100644 --- a/pronto.gemspec +++ b/pronto.gemspec @@ -31,4 +31,5 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'octokit', '~> 2.7', '>= 2.7.1' s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.0' s.add_development_dependency 'rspec', '~> 2.14', '>= 2.14.0' + s.add_development_dependency 'pry', '~> 0.9', '>= 0.9.0' end From e3298189cfcea72d2e968dded0c2f3dd7b959ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 13 Jul 2014 21:47:00 +0300 Subject: [PATCH 06/30] Use Rugged::Blame to find the commit line Requires a couple of hacks due to regressions in rugged 0.21.0 --- lib/pronto.rb | 2 ++ lib/pronto/rugged/diff/line.rb | 16 ++++++---------- lib/pronto/rugged/patch.rb | 8 +++++++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/pronto.rb b/lib/pronto.rb index c38ddefe..57a05e9b 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -18,6 +18,8 @@ module Pronto def self.run(commit = 'master', repo_path = '.', formatter = nil) repo = Rugged::Repository.new(repo_path) + # TODO: Remove this hack when regression is fixed + Rugged::Tree.define_singleton_method(:repo) { repo } commit ||= 'master' merge_base = repo.merge_base(commit, repo.head.target) patches = repo.diff(merge_base, repo.head.target) diff --git a/lib/pronto/rugged/diff/line.rb b/lib/pronto/rugged/diff/line.rb index b4652348..687d88da 100644 --- a/lib/pronto/rugged/diff/line.rb +++ b/lib/pronto/rugged/diff/line.rb @@ -2,7 +2,7 @@ module Rugged class Diff class Line def patch - hunk.owner + hunk.delta end def position @@ -19,9 +19,7 @@ def commit end def commit_sha - @commit_sha ||= begin - blameline.commit.id if blameline - end + blame[:final_commit_id] if blame end def commit_line @@ -33,7 +31,7 @@ def commit_line end lines = commit_patch ? commit_patch.lines : [] - result = lines.find { |l| blameline.lineno == l.new_lineno } + result = lines.find { |l| blame[:final_start_line_number] == l.new_lineno } result || self # no commit_line means that it was just added end @@ -52,11 +50,9 @@ def repo patch.diff.tree.repo end - def blameline - @blameline ||= begin - blamelines = repo.blame(patch.new_file_full_path).lines - blamelines.find { |line| line.lineno == new_lineno } - end + def blame + @blame ||= Blame.new(repo, patch.delta.new_file[:path], + min_line: new_lineno, max_line: new_lineno)[0] end end end diff --git a/lib/pronto/rugged/patch.rb b/lib/pronto/rugged/patch.rb index 3700ec9f..9727026b 100644 --- a/lib/pronto/rugged/patch.rb +++ b/lib/pronto/rugged/patch.rb @@ -21,7 +21,13 @@ def new_file_full_path end def lines - map(&:lines).flatten.compact + map do |hunk| + hunk.lines.map do |line| + # TODO: Remove this hack when regression is fixed + line.define_singleton_method(:hunk) { hunk } + line + end + end.flatten.compact end end end From 5a1d93da8b6072f0d82bc2b4bdc9cda127d6f671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 27 Jul 2014 18:43:53 +0300 Subject: [PATCH 07/30] Bump octokit from 2.7.0 to 3.2.0 --- pronto.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pronto.gemspec b/pronto.gemspec index 01fb5cd4..c02c84c4 100644 --- a/pronto.gemspec +++ b/pronto.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'rugged', '~> 0.21', '>= 0.21.0' s.add_runtime_dependency 'thor', '~> 0.19', '>= 0.19.1' - s.add_runtime_dependency 'octokit', '~> 2.7', '>= 2.7.1' + s.add_runtime_dependency 'octokit', '~> 3.2', '>= 3.2.0' s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.0' s.add_development_dependency 'rspec', '~> 2.14', '>= 2.14.0' s.add_development_dependency 'pry', '~> 0.9', '>= 0.9.0' From 27895e6beb3059341b22f004e5918eb822c9877b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 27 Jul 2014 18:44:20 +0300 Subject: [PATCH 08/30] Upgrade rspec to 3.0.0 --- pronto.gemspec | 3 ++- spec/pronto/rugged/diff/patch_spec.rb | 30 --------------------------- spec/pronto/rugged/remote_spec.rb | 2 +- spec/pronto/runner_spec.rb | 8 +++---- spec/spec_helper.rb | 7 +++++-- 5 files changed, 12 insertions(+), 38 deletions(-) delete mode 100644 spec/pronto/rugged/diff/patch_spec.rb diff --git a/pronto.gemspec b/pronto.gemspec index c02c84c4..e7f81f17 100644 --- a/pronto.gemspec +++ b/pronto.gemspec @@ -30,6 +30,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'thor', '~> 0.19', '>= 0.19.1' s.add_runtime_dependency 'octokit', '~> 3.2', '>= 3.2.0' s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.0' - s.add_development_dependency 'rspec', '~> 2.14', '>= 2.14.0' + s.add_development_dependency 'rspec', '~> 3.0', '>= 3.0.0' + s.add_development_dependency 'rspec-its', '~> 1.0.1', '>= 1.0.1' s.add_development_dependency 'pry', '~> 0.9', '>= 0.9.0' end diff --git a/spec/pronto/rugged/diff/patch_spec.rb b/spec/pronto/rugged/diff/patch_spec.rb deleted file mode 100644 index f9aba051..00000000 --- a/spec/pronto/rugged/diff/patch_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'spec_helper' - -module Rugged - class Diff - describe Patch do - let(:diff) { repository.diff('88558b7', '88558b7~5') } - let(:patch) { diff.patches.last } - - describe '#new_file_full_path' do - subject { patch.new_file_full_path.to_s } - it { should end_with '/pronto/spec/pronto/rugged/diff/patch_spec.rb' } - end - - describe '#lines' do - subject { patch.lines } - its(:count) { should == 28 } - end - - describe '#added_lines' do - subject { patch.added_lines } - its(:count) { should == 4 } - end - - describe '#deleted_lines' do - subject { patch.deleted_lines } - its(:count) { should == 4 } - end - end - end -end diff --git a/spec/pronto/rugged/remote_spec.rb b/spec/pronto/rugged/remote_spec.rb index 47206aaf..fe80e27b 100644 --- a/spec/pronto/rugged/remote_spec.rb +++ b/spec/pronto/rugged/remote_spec.rb @@ -2,7 +2,7 @@ module Rugged describe Remote do - let(:remote) { Remote.new(repository, url) } + let(:remote) { repository.remotes.create_anonymous(url) } describe '#github_slug' do subject { remote.github_slug } diff --git a/spec/pronto/runner_spec.rb b/spec/pronto/runner_spec.rb index cada3878..0d87b514 100644 --- a/spec/pronto/runner_spec.rb +++ b/spec/pronto/runner_spec.rb @@ -9,12 +9,12 @@ module Pronto context 'ending with .rb' do let(:path) { 'test.rb' } - it { should be_true } + it { should be_truthy } end context 'ending with .rb in directory' do let(:path) { 'amazing/test.rb' } - it { should be_true } + it { should be_truthy } end context 'executable' do @@ -23,12 +23,12 @@ module Pronto context 'ruby' do let(:shebang) { '#!ruby' } - it { should be_true } + it { should be_truthy } end context 'bash' do let(:shebang) { '#! bash' } - it { should be_false } + it { should be_falsy } end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index afabd2b1..b2e08334 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,9 +1,12 @@ require 'rspec' +require 'rspec/its' +require 'pry' + require 'pronto' RSpec.configure do |config| - config.color_enabled = true - config.treat_symbols_as_metadata_keys_with_true_values = true + config.expect_with(:rspec) { |c| c.syntax = :should } + config.mock_with(:rspec) { |c| c.syntax = :should } end def repository From 16e68afcd86a3aac9c8aea0597e2cb1f7eb3a424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 27 Jul 2014 22:11:03 +0300 Subject: [PATCH 09/30] To find commit location, use orig_ instead of final_start_line_number --- lib/pronto/rugged/diff/line.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pronto/rugged/diff/line.rb b/lib/pronto/rugged/diff/line.rb index 687d88da..90b505b9 100644 --- a/lib/pronto/rugged/diff/line.rb +++ b/lib/pronto/rugged/diff/line.rb @@ -31,7 +31,7 @@ def commit_line end lines = commit_patch ? commit_patch.lines : [] - result = lines.find { |l| blame[:final_start_line_number] == l.new_lineno } + result = lines.find { |l| blame[:orig_start_line_number] == l.new_lineno } result || self # no commit_line means that it was just added end From 6b09caa7ed6140e871118260a1af5135feedb073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 27 Jul 2014 22:34:14 +0300 Subject: [PATCH 10/30] Use additional tracking options for more accurate blaming --- lib/pronto/rugged/diff/line.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pronto/rugged/diff/line.rb b/lib/pronto/rugged/diff/line.rb index 90b505b9..8b139c8b 100644 --- a/lib/pronto/rugged/diff/line.rb +++ b/lib/pronto/rugged/diff/line.rb @@ -52,7 +52,9 @@ def repo def blame @blame ||= Blame.new(repo, patch.delta.new_file[:path], - min_line: new_lineno, max_line: new_lineno)[0] + min_line: new_lineno, max_line: new_lineno, + track_copies_same_file: true, + track_copies_any_commit_copies: true)[0] end end end From 90f5bb7830d7c3d93cebc964741b268eccb1c499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 29 Jul 2014 22:24:46 +0300 Subject: [PATCH 11/30] Get rid of Rugged monkey patches, move them to Pronto::Git namespace --- lib/pronto.rb | 11 ++-- lib/pronto/git/line.rb | 86 ++++++++++++++++++++++++++++ lib/pronto/git/patch.rb | 44 ++++++++++++++ lib/pronto/rugged/commit.rb | 8 --- lib/pronto/rugged/diff/delta.rb | 16 ------ lib/pronto/rugged/diff/line.rb | 61 -------------------- lib/pronto/rugged/patch.rb | 33 ----------- spec/pronto/git/line_spec.rb | 16 ++++++ spec/pronto/rugged/diff/line_spec.rb | 16 ------ 9 files changed, 150 insertions(+), 141 deletions(-) create mode 100644 lib/pronto/git/line.rb create mode 100644 lib/pronto/git/patch.rb delete mode 100644 lib/pronto/rugged/commit.rb delete mode 100644 lib/pronto/rugged/diff/delta.rb delete mode 100644 lib/pronto/rugged/diff/line.rb delete mode 100644 lib/pronto/rugged/patch.rb create mode 100644 spec/pronto/git/line_spec.rb diff --git a/lib/pronto.rb b/lib/pronto.rb index 57a05e9b..fc17346a 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -1,9 +1,7 @@ require 'rugged' -require 'pronto/rugged/patch' -require 'pronto/rugged/diff/delta' -require 'pronto/rugged/diff/line' +require 'pronto/git/patch' +require 'pronto/git/line' require 'pronto/rugged/remote' -require 'pronto/rugged/commit' require 'pronto/plugin' require 'pronto/message' @@ -18,11 +16,10 @@ module Pronto def self.run(commit = 'master', repo_path = '.', formatter = nil) repo = Rugged::Repository.new(repo_path) - # TODO: Remove this hack when regression is fixed - Rugged::Tree.define_singleton_method(:repo) { repo } commit ||= 'master' merge_base = repo.merge_base(commit, repo.head.target) - patches = repo.diff(merge_base, repo.head.target) + # TODO This could be cleaner + patches = repo.diff(merge_base, repo.head.target).map { |patch| Git::Patch.new(patch, repo) } result = run_all_runners(patches, merge_base) diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb new file mode 100644 index 00000000..841ca27d --- /dev/null +++ b/lib/pronto/git/line.rb @@ -0,0 +1,86 @@ +module Pronto + module Git + class Line < Struct.new(:line, :patch, :hunk) + def addition? + line.addition? + end + + def deletion? + line.deletion? + end + + def content + line.content + end + + def new_lineno + line.new_lineno + end + + def old_lineno + line.old_lineno + end + + def line_origin + line.line_origin + end + + def position + hunk_index = patch.hunks.find_index { |h| h.header == hunk.header } + line_index = patch.lines.find_index(line) + + line_index + hunk_index + 1 + end + + def commit + @commit ||= begin + repo.lookup(commit_sha) if commit_sha + end + end + + def commit_sha + blame[:final_commit_id] if blame + end + + def commit_line + @commit_line ||= begin + # TODO: Rugged does not seem to support diffing against multiple parents + diff = commit.diff(reverse: true) unless commit.parents.count != 1 + patches = diff ? diff.patches : [] + + # TODO This could be cleaner + patches = patches.map { |patch| Git::Patch.new(patch, repo) } + + commit_patch = patches.find do |p| + patch.new_file_full_path == p.new_file_full_path + end + + lines = commit_patch ? commit_patch.lines : [] + result = lines.find { |l| blame[:orig_start_line_number] == l.new_lineno } + + result || line # no commit_line means that it was just added + end + end + + def ==(other) + line.content == other.content && + line_origin == other.line_origin && + old_lineno == other.old_lineno && + new_lineno == other.new_lineno + end + + private + + def repo + patch.repo + end + + def blame + @blame ||= Rugged::Blame.new(repo, patch.delta.new_file[:path], + min_line: new_lineno, max_line: new_lineno, + track_copies_same_file: true, + track_copies_any_commit_copies: true)[0] + end + end + end +end diff --git a/lib/pronto/git/patch.rb b/lib/pronto/git/patch.rb new file mode 100644 index 00000000..dc9e8cc9 --- /dev/null +++ b/lib/pronto/git/patch.rb @@ -0,0 +1,44 @@ +require 'pathname' + +module Pronto + module Git + class Patch < Struct.new(:patch, :repo) + def delta + patch.delta + end + + def hunks + patch.hunks + end + + def additions + patch.stat[0] + end + + def deletions + patch.stat[1] + end + + def lines + @lines ||= begin + patch.map do |hunk| + hunk.lines.map { |line| Line.new(line, self, hunk) } + end.flatten.compact + end + end + + def added_lines + lines.select(&:addition?) + end + + def deleted_lines + lines.select(&:deletion?) + end + + def new_file_full_path + repo_path = Pathname.new(repo.path).parent + repo_path.join(patch.delta.new_file[:path]) + end + end + end +end diff --git a/lib/pronto/rugged/commit.rb b/lib/pronto/rugged/commit.rb deleted file mode 100644 index 60f14bcc..00000000 --- a/lib/pronto/rugged/commit.rb +++ /dev/null @@ -1,8 +0,0 @@ -module Rugged - class Commit - def show - # TODO: Rugged does not seem to support diffing against multiple parents - diff(reverse: true) unless parents.count != 1 - end - end -end diff --git a/lib/pronto/rugged/diff/delta.rb b/lib/pronto/rugged/diff/delta.rb deleted file mode 100644 index 32f48624..00000000 --- a/lib/pronto/rugged/diff/delta.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'pathname' - -module Rugged - class Diff - class Delta - def repo - diff.tree.repo - end - - def new_file_full_path - repo_path = Pathname.new(repo.path).parent - repo_path.join(new_file[:path]) - end - end - end -end diff --git a/lib/pronto/rugged/diff/line.rb b/lib/pronto/rugged/diff/line.rb deleted file mode 100644 index 8b139c8b..00000000 --- a/lib/pronto/rugged/diff/line.rb +++ /dev/null @@ -1,61 +0,0 @@ -module Rugged - class Diff - class Line - def patch - hunk.delta - end - - def position - hunk_index = patch.hunks.find_index { |h| h.header == hunk.header } - line_index = patch.lines.find_index(self) - - line_index + hunk_index + 1 - end - - def commit - @commit ||= begin - repo.lookup(commit_sha) if commit_sha - end - end - - def commit_sha - blame[:final_commit_id] if blame - end - - def commit_line - @commit_line ||= begin - diff = commit.show - patches = diff ? diff.patches : [] - commit_patch = patches.find do |p| - patch.new_file_full_path == p.new_file_full_path - end - - lines = commit_patch ? commit_patch.lines : [] - result = lines.find { |l| blame[:orig_start_line_number] == l.new_lineno } - - result || self # no commit_line means that it was just added - end - end - - def ==(other) - content == other.content && - line_origin == other.line_origin && - old_lineno == other.old_lineno && - new_lineno == other.new_lineno - end - - private - - def repo - patch.diff.tree.repo - end - - def blame - @blame ||= Blame.new(repo, patch.delta.new_file[:path], - min_line: new_lineno, max_line: new_lineno, - track_copies_same_file: true, - track_copies_any_commit_copies: true)[0] - end - end - end -end diff --git a/lib/pronto/rugged/patch.rb b/lib/pronto/rugged/patch.rb deleted file mode 100644 index 9727026b..00000000 --- a/lib/pronto/rugged/patch.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Rugged - class Patch - def additions - stat[0] - end - - def deletions - stat[1] - end - - def added_lines - lines.select(&:addition?) - end - - def deleted_lines - lines.select(&:deletion?) - end - - def new_file_full_path - delta.new_file_full_path - end - - def lines - map do |hunk| - hunk.lines.map do |line| - # TODO: Remove this hack when regression is fixed - line.define_singleton_method(:hunk) { hunk } - line - end - end.flatten.compact - end - end -end diff --git a/spec/pronto/git/line_spec.rb b/spec/pronto/git/line_spec.rb new file mode 100644 index 00000000..2eb60913 --- /dev/null +++ b/spec/pronto/git/line_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +module Pronto + module Git + describe Line do + let(:diff) { repository.diff('88558b7', '88558b7~5') } + let(:patch) { Patch.new(diff.patches.last, repository) } + let(:line) { patch.lines[2] } + + describe '#position' do + subject { line.position } + it { should == 3 } + end + end + end +end diff --git a/spec/pronto/rugged/diff/line_spec.rb b/spec/pronto/rugged/diff/line_spec.rb index 984accb6..e69de29b 100644 --- a/spec/pronto/rugged/diff/line_spec.rb +++ b/spec/pronto/rugged/diff/line_spec.rb @@ -1,16 +0,0 @@ -require 'spec_helper' - -module Rugged - class Diff - describe Line do - let(:diff) { repository.diff('88558b7', '88558b7~5') } - let(:patch) { diff.patches.last } - let(:line) { patch.lines[2] } - - describe '#position' do - subject { line.position } - it { should == 3 } - end - end - end -end From 064f42f3fb21e78767274308e3c38f2c9861d056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Wed, 30 Jul 2014 21:33:55 +0300 Subject: [PATCH 12/30] Extract Git::Patches, Git::Repository and Git::Remote --- lib/pronto.rb | 20 ++++---- lib/pronto/git/line.rb | 17 ++----- lib/pronto/git/patches.rb | 18 +++++++ lib/pronto/git/remote.rb | 10 ++++ lib/pronto/git/repository.rb | 50 +++++++++++++++++++ lib/pronto/message.rb | 2 +- lib/pronto/rugged/remote.rb | 8 --- .../formatter/github_formattter_spec.rb | 1 - spec/pronto/formatter/json_formatter_spec.rb | 1 - spec/pronto/formatter/text_formatter_spec.rb | 1 - spec/pronto/git/line_spec.rb | 2 +- spec/pronto/git/remote_spec.rb | 23 +++++++++ spec/pronto/rugged/diff/line_spec.rb | 0 spec/pronto/rugged/remote_spec.rb | 21 -------- spec/pronto_spec.rb | 4 +- spec/spec_helper.rb | 3 +- 16 files changed, 121 insertions(+), 60 deletions(-) create mode 100644 lib/pronto/git/patches.rb create mode 100644 lib/pronto/git/remote.rb create mode 100644 lib/pronto/git/repository.rb delete mode 100644 lib/pronto/rugged/remote.rb create mode 100644 spec/pronto/git/remote_spec.rb delete mode 100644 spec/pronto/rugged/diff/line_spec.rb delete mode 100644 spec/pronto/rugged/remote_spec.rb diff --git a/lib/pronto.rb b/lib/pronto.rb index fc17346a..ff21c938 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -1,7 +1,10 @@ require 'rugged' + +require 'pronto/git/repository' +require 'pronto/git/patches' require 'pronto/git/patch' require 'pronto/git/line' -require 'pronto/rugged/remote' +require 'pronto/git/remote' require 'pronto/plugin' require 'pronto/message' @@ -15,13 +18,12 @@ module Pronto def self.run(commit = 'master', repo_path = '.', formatter = nil) - repo = Rugged::Repository.new(repo_path) commit ||= 'master' - merge_base = repo.merge_base(commit, repo.head.target) - # TODO This could be cleaner - patches = repo.diff(merge_base, repo.head.target).map { |patch| Git::Patch.new(patch, repo) } - result = run_all_runners(patches, merge_base) + repo = Git::Repository.new(repo_path) + patches = repo.diff(commit) + + result = run_all_runners(patches) formatter ||= default_formatter formatter.format(result, repo) @@ -33,7 +35,7 @@ def self.gem_names true elsif gem.name != 'pronto' runner_path = File.join(gem.full_gem_path, "lib/pronto/#{gem.name}.rb") - File.exists?(runner_path) + File.exist?(runner_path) end end @@ -44,9 +46,9 @@ def self.gem_names private - def self.run_all_runners(patches, commit) + def self.run_all_runners(patches) Runner.runners.map do |runner| - runner.new.run(patches, commit) + runner.new.run(patches, patches.commit) end.flatten.compact end diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb index 841ca27d..aaae7309 100644 --- a/lib/pronto/git/line.rb +++ b/lib/pronto/git/line.rb @@ -32,24 +32,13 @@ def position line_index + hunk_index + 1 end - def commit - @commit ||= begin - repo.lookup(commit_sha) if commit_sha - end - end - def commit_sha blame[:final_commit_id] if blame end def commit_line @commit_line ||= begin - # TODO: Rugged does not seem to support diffing against multiple parents - diff = commit.diff(reverse: true) unless commit.parents.count != 1 - patches = diff ? diff.patches : [] - - # TODO This could be cleaner - patches = patches.map { |patch| Git::Patch.new(patch, repo) } + patches = repo.show_commit(commit_sha) commit_patch = patches.find do |p| patch.new_file_full_path == p.new_file_full_path @@ -58,7 +47,7 @@ def commit_line lines = commit_patch ? commit_patch.lines : [] result = lines.find { |l| blame[:orig_start_line_number] == l.new_lineno } - result || line # no commit_line means that it was just added + result || self # no commit_line means that it was just added end end @@ -76,7 +65,7 @@ def repo end def blame - @blame ||= Rugged::Blame.new(repo, patch.delta.new_file[:path], + @blame ||= Rugged::Blame.new(repo.rugged, patch.delta.new_file[:path], min_line: new_lineno, max_line: new_lineno, track_copies_same_file: true, track_copies_any_commit_copies: true)[0] diff --git a/lib/pronto/git/patches.rb b/lib/pronto/git/patches.rb new file mode 100644 index 00000000..041b6cb4 --- /dev/null +++ b/lib/pronto/git/patches.rb @@ -0,0 +1,18 @@ +module Pronto + module Git + class Patches + include Enumerable + + attr_reader :commit, :repo + + def initialize(repo, commit, patches) + @commit = commit + @patches = patches.map { |patch| Git::Patch.new(patch, repo) } + end + + def each(&block) + @patches.each(&block) + end + end + end +end diff --git a/lib/pronto/git/remote.rb b/lib/pronto/git/remote.rb new file mode 100644 index 00000000..d837e837 --- /dev/null +++ b/lib/pronto/git/remote.rb @@ -0,0 +1,10 @@ +module Pronto + module Git + class Remote < Struct.new(:remote) + def github_slug + match = /.*github.com(:|\/)(?.*).git/.match(remote.url) + match[:slug] if match + end + end + end +end diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb new file mode 100644 index 00000000..4d76cd67 --- /dev/null +++ b/lib/pronto/git/repository.rb @@ -0,0 +1,50 @@ +module Pronto + module Git + class Repository + def initialize(path) + @repo = Rugged::Repository.new(path) + end + + def remotes + @repo.remotes.map { |remote| Remote.new(remote) } + end + + def diff(commit) + merge_base = merge_base(commit) + patches = @repo.diff(merge_base, head) + Patches.new(self, merge_base, patches) + end + + def show_commit(sha) + return [] unless sha + + commit = @repo.lookup(sha) + return [] if commit.parents.count != 1 + + # TODO: Rugged does not seem to support diffing against multiple parents + diff = commit.diff(reverse: true) + return [] if diff.nil? + + Patches.new(self, sha, diff.patches) + end + + def path + @repo.path + end + + def rugged + @repo + end + + private + + def merge_base(commit) + @repo.merge_base(commit, head) + end + + def head + @repo.head.target + end + end + end +end diff --git a/lib/pronto/message.rb b/lib/pronto/message.rb index 33dbe362..a8fcafa6 100644 --- a/lib/pronto/message.rb +++ b/lib/pronto/message.rb @@ -18,7 +18,7 @@ def initialize(path, line, level, msg, commit_sha = nil) end def repo - line.patch.delta.repo if line + line.patch.repo if line end end end diff --git a/lib/pronto/rugged/remote.rb b/lib/pronto/rugged/remote.rb deleted file mode 100644 index 4f05cddd..00000000 --- a/lib/pronto/rugged/remote.rb +++ /dev/null @@ -1,8 +0,0 @@ -module Rugged - class Remote - def github_slug - match = /.*github.com(:|\/)(?.*).git/.match(url) - match[:slug] if match - end - end -end diff --git a/spec/pronto/formatter/github_formattter_spec.rb b/spec/pronto/formatter/github_formattter_spec.rb index 0c9f10b6..faf5759a 100644 --- a/spec/pronto/formatter/github_formattter_spec.rb +++ b/spec/pronto/formatter/github_formattter_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'ostruct' module Pronto module Formatter diff --git a/spec/pronto/formatter/json_formatter_spec.rb b/spec/pronto/formatter/json_formatter_spec.rb index bb12b3f3..bf2498b9 100644 --- a/spec/pronto/formatter/json_formatter_spec.rb +++ b/spec/pronto/formatter/json_formatter_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'ostruct' module Pronto module Formatter diff --git a/spec/pronto/formatter/text_formatter_spec.rb b/spec/pronto/formatter/text_formatter_spec.rb index a921d13a..e4912e04 100644 --- a/spec/pronto/formatter/text_formatter_spec.rb +++ b/spec/pronto/formatter/text_formatter_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'ostruct' module Pronto module Formatter diff --git a/spec/pronto/git/line_spec.rb b/spec/pronto/git/line_spec.rb index 2eb60913..d8637f49 100644 --- a/spec/pronto/git/line_spec.rb +++ b/spec/pronto/git/line_spec.rb @@ -3,7 +3,7 @@ module Pronto module Git describe Line do - let(:diff) { repository.diff('88558b7', '88558b7~5') } + let(:diff) { repository.rugged.diff('88558b7', '88558b7~5') } let(:patch) { Patch.new(diff.patches.last, repository) } let(:line) { patch.lines[2] } diff --git a/spec/pronto/git/remote_spec.rb b/spec/pronto/git/remote_spec.rb new file mode 100644 index 00000000..e2452df4 --- /dev/null +++ b/spec/pronto/git/remote_spec.rb @@ -0,0 +1,23 @@ +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/rugged/diff/line_spec.rb b/spec/pronto/rugged/diff/line_spec.rb deleted file mode 100644 index e69de29b..00000000 diff --git a/spec/pronto/rugged/remote_spec.rb b/spec/pronto/rugged/remote_spec.rb deleted file mode 100644 index fe80e27b..00000000 --- a/spec/pronto/rugged/remote_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'spec_helper' - -module Rugged - describe Remote do - let(:remote) { repository.remotes.create_anonymous(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 diff --git a/spec/pronto_spec.rb b/spec/pronto_spec.rb index b9c2e0b5..5cf0f294 100644 --- a/spec/pronto_spec.rb +++ b/spec/pronto_spec.rb @@ -21,7 +21,7 @@ context 'with good path' do let(:gems) { [double(name: 'good', full_gem_path: '/good')] } before do - File.stub(:exists?).with('/good/lib/pronto/good.rb').and_return(true) + File.stub(:exist?).with('/good/lib/pronto/good.rb').and_return(true) end it { should include('good') } end @@ -29,7 +29,7 @@ context 'with bad path' do let(:gems) { [double(name: 'bad', full_gem_path: '/bad')] } before do - File.stub(:exists?).with('/bad/lib/pronto/bad.rb').and_return(false) + File.stub(:exist?).with('/bad/lib/pronto/bad.rb').and_return(false) end it { should_not include('bad') } end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b2e08334..dcede2e2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ require 'rspec' require 'rspec/its' require 'pry' +require 'ostruct' require 'pronto' @@ -10,7 +11,7 @@ end def repository - Rugged::Repository.init_at('.') + Pronto::Git::Repository.new('.') end def load_fixture(fixture_name) From 27ed09e0499118c265f38aa01838a28c5c869e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Wed, 30 Jul 2014 22:33:25 +0300 Subject: [PATCH 13/30] Move #blame to Patch from Line --- lib/pronto/git/line.rb | 11 ++--------- lib/pronto/git/patch.rb | 4 ++++ lib/pronto/git/repository.rb | 7 +++++-- spec/pronto/formatter/github_formattter_spec.rb | 1 + spec/pronto/git/line_spec.rb | 16 ---------------- spec/spec_helper.rb | 4 ---- 6 files changed, 12 insertions(+), 31 deletions(-) delete mode 100644 spec/pronto/git/line_spec.rb diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb index aaae7309..d6e4803f 100644 --- a/lib/pronto/git/line.rb +++ b/lib/pronto/git/line.rb @@ -38,7 +38,7 @@ def commit_sha def commit_line @commit_line ||= begin - patches = repo.show_commit(commit_sha) + patches = patch.repo.show_commit(commit_sha) commit_patch = patches.find do |p| patch.new_file_full_path == p.new_file_full_path @@ -60,15 +60,8 @@ def ==(other) private - def repo - patch.repo - end - def blame - @blame ||= Rugged::Blame.new(repo.rugged, patch.delta.new_file[:path], - min_line: new_lineno, max_line: new_lineno, - track_copies_same_file: true, - track_copies_any_commit_copies: true)[0] + @blame ||= patch.blame(new_lineno) end end end diff --git a/lib/pronto/git/patch.rb b/lib/pronto/git/patch.rb index dc9e8cc9..1b4eb9b9 100644 --- a/lib/pronto/git/patch.rb +++ b/lib/pronto/git/patch.rb @@ -19,6 +19,10 @@ def deletions patch.stat[1] end + def blame(lineno) + repo.blame(self, lineno) + end + def lines @lines ||= begin patch.map do |hunk| diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb index 4d76cd67..35c5196a 100644 --- a/lib/pronto/git/repository.rb +++ b/lib/pronto/git/repository.rb @@ -32,8 +32,11 @@ def path @repo.path end - def rugged - @repo + def blame(patch, lineno) + Rugged::Blame.new(@repo, patch.delta.new_file[:path], + min_line: lineno, max_line: lineno, + track_copies_same_file: true, + track_copies_any_commit_copies: true)[0] end private diff --git a/spec/pronto/formatter/github_formattter_spec.rb b/spec/pronto/formatter/github_formattter_spec.rb index faf5759a..3236fe37 100644 --- a/spec/pronto/formatter/github_formattter_spec.rb +++ b/spec/pronto/formatter/github_formattter_spec.rb @@ -8,6 +8,7 @@ module Formatter describe '#format' do subject { github_formatter.format(messages, repository) } let(:messages) { [message, message] } + let(:repository) { Pronto::Git::Repository.new('.') } let(:message) { Message.new('path/to', line, :warning, 'crucial') } let(:line) { OpenStruct.new(new_lineno: 1) } before { line.stub(:commit_line).and_return(line) } diff --git a/spec/pronto/git/line_spec.rb b/spec/pronto/git/line_spec.rb deleted file mode 100644 index d8637f49..00000000 --- a/spec/pronto/git/line_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -module Pronto - module Git - describe Line do - let(:diff) { repository.rugged.diff('88558b7', '88558b7~5') } - let(:patch) { Patch.new(diff.patches.last, repository) } - let(:line) { patch.lines[2] } - - describe '#position' do - subject { line.position } - it { should == 3 } - end - end - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dcede2e2..efe37dfa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,10 +10,6 @@ config.mock_with(:rspec) { |c| c.syntax = :should } end -def repository - Pronto::Git::Repository.new('.') -end - def load_fixture(fixture_name) path = File.join(%w(spec support files), fixture_name) File.read(path).strip From 6a7c00108e4b7d2988dec7c5b12c4b356dd6ab9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 15:56:41 +0300 Subject: [PATCH 14/30] Some specs for Git::Line --- spec/pronto/git/line_spec.rb | 80 ++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 spec/pronto/git/line_spec.rb diff --git a/spec/pronto/git/line_spec.rb b/spec/pronto/git/line_spec.rb new file mode 100644 index 00000000..24f663b9 --- /dev/null +++ b/spec/pronto/git/line_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +module Pronto + module Git + describe Line do + let(:line) { Line.new(rugged_line, patch, hunk) } + + let(:patch) { nil } + let(:hunk) { nil } + + describe '#addition?' do + subject { line.addition? } + + let(:rugged_line) { double(addition?: true) } + it { should == true } + end + + describe '#deletion?' do + subject { line.deletion? } + + let(:rugged_line) { double(deletion?: false) } + it { should == false } + end + + describe '#content' do + subject { line.content } + + let(:rugged_line) { double(content: 'hello') } + it { should == 'hello' } + end + + describe '#new_lineno' do + subject { line.new_lineno } + + let(:rugged_line) { double(new_lineno: 1) } + it { should == 1 } + end + + describe '#old_lineno' do + subject { line.old_lineno } + + let(:rugged_line) { double(old_lineno: 42) } + it { should == 42 } + end + + describe '#line_origin' do + subject { line.line_origin } + + let(:rugged_line) { double(line_origin: 15) } + it { should == 15 } + end + + describe '#==' do + subject { line == other } + + let(:rugged_line) do + double(content: 'hello', + line_origin: 2, + new_lineno: 10, + old_lineno: 11) + end + + context 'equal' do + let(:other) { rugged_line } + it { should == true } + end + + context 'different' do + let(:other) do + double(content: 'Bob', + line_origin: 7, + new_lineno: 17, + old_lineno: 17) + end + it { should == false } + end + end + end + end +end From ed3688b94cf33a49b023c8d9b6c5973dbeb94c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 16:03:20 +0300 Subject: [PATCH 15/30] Use Forwardable in Git::Line --- lib/pronto/git/line.rb | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb index d6e4803f..0f495471 100644 --- a/lib/pronto/git/line.rb +++ b/lib/pronto/git/line.rb @@ -1,29 +1,12 @@ +require 'forwardable' + module Pronto module Git class Line < Struct.new(:line, :patch, :hunk) - def addition? - line.addition? - end - - def deletion? - line.deletion? - end - - def content - line.content - end + extend Forwardable - def new_lineno - line.new_lineno - end - - def old_lineno - line.old_lineno - end - - def line_origin - line.line_origin - end + def_delegators :line, :addition?, :deletion?, :content, :new_lineno, + :old_lineno, :line_origin def position hunk_index = patch.hunks.find_index { |h| h.header == hunk.header } @@ -52,7 +35,7 @@ def commit_line end def ==(other) - line.content == other.content && + content == other.content && line_origin == other.line_origin && old_lineno == other.old_lineno && new_lineno == other.new_lineno From 565345e3b3ec92a537eece44d2d6654d16671126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 16:06:36 +0300 Subject: [PATCH 16/30] Some specs for Git::Patch --- spec/pronto/git/patch_spec.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 spec/pronto/git/patch_spec.rb diff --git a/spec/pronto/git/patch_spec.rb b/spec/pronto/git/patch_spec.rb new file mode 100644 index 00000000..7a383d5f --- /dev/null +++ b/spec/pronto/git/patch_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +module Pronto + module Git + describe Patch do + let(:patch) { Patch.new(rugged_patch, repo) } + + let(:repo) { nil } + + describe '#additions' do + subject { patch.additions } + + let(:rugged_patch) { double(stat: [15, 13]) } + it { should == 15 } + end + + describe '#deletions' do + subject { patch.deletions } + + let(:rugged_patch) { double(stat: [5, 17]) } + it { should == 17 } + end + end + end +end From befc3e76c9132d07af414c9caf3c7defddc6f709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 16:08:41 +0300 Subject: [PATCH 17/30] Use Forwardable in Git::Patch --- lib/pronto.rb | 1 + lib/pronto/git/line.rb | 2 -- lib/pronto/git/patch.rb | 16 ++++++---------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/pronto.rb b/lib/pronto.rb index ff21c938..0312702c 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -1,4 +1,5 @@ require 'rugged' +require 'forwardable' require 'pronto/git/repository' require 'pronto/git/patches' diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb index 0f495471..e306fc47 100644 --- a/lib/pronto/git/line.rb +++ b/lib/pronto/git/line.rb @@ -1,5 +1,3 @@ -require 'forwardable' - module Pronto module Git class Line < Struct.new(:line, :patch, :hunk) diff --git a/lib/pronto/git/patch.rb b/lib/pronto/git/patch.rb index 1b4eb9b9..268634d1 100644 --- a/lib/pronto/git/patch.rb +++ b/lib/pronto/git/patch.rb @@ -3,20 +3,16 @@ module Pronto module Git class Patch < Struct.new(:patch, :repo) - def delta - patch.delta - end + extend Forwardable - def hunks - patch.hunks - end + def_delegators :patch, :delta, :hunks, :stat def additions - patch.stat[0] + stat[0] end def deletions - patch.stat[1] + stat[1] end def blame(lineno) @@ -25,7 +21,7 @@ def blame(lineno) def lines @lines ||= begin - patch.map do |hunk| + hunks.map do |hunk| hunk.lines.map { |line| Line.new(line, self, hunk) } end.flatten.compact end @@ -41,7 +37,7 @@ def deleted_lines def new_file_full_path repo_path = Pathname.new(repo.path).parent - repo_path.join(patch.delta.new_file[:path]) + repo_path.join(delta.new_file[:path]) end end end From 62c3ea9097eeb600e1d31e7d284f13ced410c072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 17:10:36 +0300 Subject: [PATCH 18/30] Replace usages of OpenStruct in specs to double() --- spec/pronto/formatter/checkstyle_formatter_spec.rb | 2 +- spec/pronto/formatter/formatter_spec.rb | 1 - spec/pronto/formatter/github_formattter_spec.rb | 2 +- spec/pronto/formatter/json_formatter_spec.rb | 2 +- spec/pronto/formatter/text_formatter_spec.rb | 2 +- spec/spec_helper.rb | 1 - 6 files changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/pronto/formatter/checkstyle_formatter_spec.rb b/spec/pronto/formatter/checkstyle_formatter_spec.rb index a917509e..43b83358 100644 --- a/spec/pronto/formatter/checkstyle_formatter_spec.rb +++ b/spec/pronto/formatter/checkstyle_formatter_spec.rb @@ -7,7 +7,7 @@ module Formatter describe '#format' do subject { checkstyle_formatter.format(messages, nil) } - let(:line) { OpenStruct.new(new_lineno: 1) } + let(:line) { double(new_lineno: 1, commit_sha: '123') } let(:error) { Message.new('path/to', line, :error, 'Line Error') } let(:warning) { Message.new('path/to', line, :warning, 'Line Warning') } let(:messages) { [error, warning] } diff --git a/spec/pronto/formatter/formatter_spec.rb b/spec/pronto/formatter/formatter_spec.rb index 8ea49185..43372e61 100644 --- a/spec/pronto/formatter/formatter_spec.rb +++ b/spec/pronto/formatter/formatter_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'ostruct' module Pronto module Formatter diff --git a/spec/pronto/formatter/github_formattter_spec.rb b/spec/pronto/formatter/github_formattter_spec.rb index 3236fe37..ed293d15 100644 --- a/spec/pronto/formatter/github_formattter_spec.rb +++ b/spec/pronto/formatter/github_formattter_spec.rb @@ -10,7 +10,7 @@ module Formatter let(:messages) { [message, message] } let(:repository) { Pronto::Git::Repository.new('.') } let(:message) { Message.new('path/to', line, :warning, 'crucial') } - let(:line) { OpenStruct.new(new_lineno: 1) } + let(:line) { double(new_lineno: 1, commit_sha: '123', position: nil) } before { line.stub(:commit_line).and_return(line) } specify do diff --git a/spec/pronto/formatter/json_formatter_spec.rb b/spec/pronto/formatter/json_formatter_spec.rb index bf2498b9..f5c68374 100644 --- a/spec/pronto/formatter/json_formatter_spec.rb +++ b/spec/pronto/formatter/json_formatter_spec.rb @@ -9,7 +9,7 @@ module Formatter subject { json_formatter.format(messages, nil) } let(:messages) { [message, message] } let(:message) { Message.new('path/to', line, :warning, 'crucial') } - let(:line) { OpenStruct.new(new_lineno: 1) } + let(:line) { double(new_lineno: 1, commit_sha: nil) } it { should == '[{"level":"W","message":"crucial","path":"path/to","line":1},{"level":"W","message":"crucial","path":"path/to","line":1}]' } diff --git a/spec/pronto/formatter/text_formatter_spec.rb b/spec/pronto/formatter/text_formatter_spec.rb index e4912e04..9a54c377 100644 --- a/spec/pronto/formatter/text_formatter_spec.rb +++ b/spec/pronto/formatter/text_formatter_spec.rb @@ -9,7 +9,7 @@ module Formatter subject { text_formatter.format(messages, nil) } let(:messages) { [message, message] } let(:message) { Message.new('path/to', line, :warning, 'crucial') } - let(:line) { OpenStruct.new(new_lineno: 1) } + let(:line) { double(new_lineno: 1, commit_sha: '123') } its(:count) { should == 2 } its(:first) { should == 'path/to:1 W: crucial' } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index efe37dfa..a1cd95c6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,6 @@ require 'rspec' require 'rspec/its' require 'pry' -require 'ostruct' require 'pronto' From b5f80b6e110279235a0aeca742feea2d1295ce79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 17:27:12 +0300 Subject: [PATCH 19/30] More pessimistic dependencies of rugged and thor --- pronto.gemspec | 31 +++++++++++++++---------------- spec/spec_helper.rb | 2 -- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/pronto.gemspec b/pronto.gemspec index e7f81f17..ec6f9ac8 100644 --- a/pronto.gemspec +++ b/pronto.gemspec @@ -4,13 +4,13 @@ $LOAD_PATH.push File.expand_path('../lib', __FILE__) require 'pronto/version' Gem::Specification.new do |s| - s.name = 'pronto' - s.version = Pronto::VERSION - s.platform = Gem::Platform::RUBY - s.author = 'Mindaugas Mozūras' - s.email = 'mindaugas.mozuras@gmail.com' - s.homepage = 'http://github.org/mmozuras/pronto' - s.summary = 'Pronto runs analysis by checking only the introduced changes' + s.name = 'pronto' + s.version = Pronto::VERSION + s.platform = Gem::Platform::RUBY + s.author = 'Mindaugas Mozūras' + s.email = 'mindaugas.mozuras@gmail.com' + s.homepage = 'http://github.org/mmozuras/pronto' + s.summary = 'Pronto runs analysis by checking only the introduced changes' s.description = <<-EOF Pronto runs analysis quickly by checking only the relevant changes. Created to be used on pull requests, but suited for other scenarios as well. Perfect @@ -21,16 +21,15 @@ Gem::Specification.new do |s| s.required_rubygems_version = '>= 1.3.6' s.license = 'MIT' - s.files = Dir.glob('{lib}/**/*') + %w(LICENSE README.md) - s.test_files = `git ls-files -- {spec}/*`.split("\n") + s.files = Dir.glob('{lib}/**/*') + %w(LICENSE README.md) + s.test_files = `git ls-files -- {spec}/*`.split("\n") s.require_paths = ['lib'] s.executables << 'pronto' - s.add_runtime_dependency 'rugged', '~> 0.21', '>= 0.21.0' - s.add_runtime_dependency 'thor', '~> 0.19', '>= 0.19.1' - s.add_runtime_dependency 'octokit', '~> 3.2', '>= 3.2.0' - s.add_development_dependency 'rake', '~> 10.1', '>= 10.1.0' - s.add_development_dependency 'rspec', '~> 3.0', '>= 3.0.0' - s.add_development_dependency 'rspec-its', '~> 1.0.1', '>= 1.0.1' - s.add_development_dependency 'pry', '~> 0.9', '>= 0.9.0' + s.add_runtime_dependency 'rugged', '~> 0.21.0' + s.add_runtime_dependency 'thor', '~> 0.19.0' + s.add_runtime_dependency 'octokit', '~> 3.2' + s.add_development_dependency 'rake', '~> 10.3' + s.add_development_dependency 'rspec', '~> 3.0' + s.add_development_dependency 'rspec-its', '~> 1.0' end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a1cd95c6..14fd02b4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,5 @@ require 'rspec' require 'rspec/its' -require 'pry' - require 'pronto' RSpec.configure do |config| From fbf8d76e493e8673f5efb876de4c0b28a65b1e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 17:42:29 +0300 Subject: [PATCH 20/30] Extract Pronto::Github to be used by multiple formatters --- lib/pronto.rb | 2 ++ lib/pronto/formatter/github_formatter.rb | 8 +------- lib/pronto/github.rb | 17 +++++++++++++++++ lib/pronto/rake_task/travis_pull_request.rb | 1 - spec/pronto/formatter/github_formattter_spec.rb | 2 +- 5 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 lib/pronto/github.rb diff --git a/lib/pronto.rb b/lib/pronto.rb index 0312702c..683486c7 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -1,4 +1,5 @@ require 'rugged' +require 'octokit' require 'forwardable' require 'pronto/git/repository' @@ -10,6 +11,7 @@ require 'pronto/plugin' require 'pronto/message' require 'pronto/runner' +require 'pronto/github' require 'pronto/formatter/text_formatter' require 'pronto/formatter/json_formatter' diff --git a/lib/pronto/formatter/github_formatter.rb b/lib/pronto/formatter/github_formatter.rb index 55022c3f..1ab21075 100644 --- a/lib/pronto/formatter/github_formatter.rb +++ b/lib/pronto/formatter/github_formatter.rb @@ -1,5 +1,3 @@ -require 'octokit' - module Pronto module Formatter class GithubFormatter @@ -33,12 +31,8 @@ def create_comment(repo, sha, position, path, body) end end - def access_token - ENV['GITHUB_ACCESS_TOKEN'] - end - def client - @client ||= Octokit::Client.new(access_token: access_token) + @client ||= Github.new end end end diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb new file mode 100644 index 00000000..f7185827 --- /dev/null +++ b/lib/pronto/github.rb @@ -0,0 +1,17 @@ +module Pronto + class Github + extend Forwardable + + def_delegators :client, :commit_comments, :create_commit_comment + + private + + def client + @client ||= Octokit::Client.new(access_token: access_token) + end + + def access_token + ENV['GITHUB_ACCESS_TOKEN'] + end + end +end diff --git a/lib/pronto/rake_task/travis_pull_request.rb b/lib/pronto/rake_task/travis_pull_request.rb index b52a6038..76d7985b 100644 --- a/lib/pronto/rake_task/travis_pull_request.rb +++ b/lib/pronto/rake_task/travis_pull_request.rb @@ -1,5 +1,4 @@ require 'pronto' -require 'octokit' require 'rake' require 'rake/tasklib' diff --git a/spec/pronto/formatter/github_formattter_spec.rb b/spec/pronto/formatter/github_formattter_spec.rb index ed293d15..0980295f 100644 --- a/spec/pronto/formatter/github_formattter_spec.rb +++ b/spec/pronto/formatter/github_formattter_spec.rb @@ -8,7 +8,7 @@ module Formatter describe '#format' do subject { github_formatter.format(messages, repository) } let(:messages) { [message, message] } - let(:repository) { Pronto::Git::Repository.new('.') } + let(:repository) { Git::Repository.new('.') } let(:message) { Message.new('path/to', line, :warning, 'crucial') } let(:line) { double(new_lineno: 1, commit_sha: '123', position: nil) } before { line.stub(:commit_line).and_return(line) } From 800277358634d138a36eb885ba4b59c03dacc4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 18:02:45 +0300 Subject: [PATCH 21/30] More specs for Git::Patch --- lib/pronto/git/patch.rb | 4 ++-- spec/pronto/git/patch_spec.rb | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/pronto/git/patch.rb b/lib/pronto/git/patch.rb index 268634d1..0c5729ee 100644 --- a/lib/pronto/git/patch.rb +++ b/lib/pronto/git/patch.rb @@ -21,9 +21,9 @@ def blame(lineno) def lines @lines ||= begin - hunks.map do |hunk| + hunks.flat_map do |hunk| hunk.lines.map { |line| Line.new(line, self, hunk) } - end.flatten.compact + end end end diff --git a/spec/pronto/git/patch_spec.rb b/spec/pronto/git/patch_spec.rb index 7a383d5f..e045fb96 100644 --- a/spec/pronto/git/patch_spec.rb +++ b/spec/pronto/git/patch_spec.rb @@ -20,6 +20,24 @@ module Git let(:rugged_patch) { double(stat: [5, 17]) } it { should == 17 } end + + describe '#lines' do + subject { patch.lines } + + let(:hunks) { [double(lines: [1, 2]), double(lines: [3])] } + let(:rugged_patch) { double(hunks: hunks) } + its(:count) { should == 3 } + end + + describe '#new_file_full_path' do + subject { patch.new_file_full_path } + + let(:rugged_patch) do + double(delta: double(new_file: { path: 'test.md' })) + end + let(:repo) { double(path: '/house/of/cards/orig.md') } + its(:to_s) { should == '/house/of/cards/test.md' } + end end end end From 0eabaa46565189f16c53b948c9ae7f4a17d166a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 3 Aug 2014 19:23:48 +0300 Subject: [PATCH 22/30] Extract comment related stuff to Pronto::Github --- lib/pronto/formatter/github_formatter.rb | 21 +++++++-------------- lib/pronto/git/repository.rb | 8 ++++++-- lib/pronto/github.rb | 19 +++++++++++++++++-- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/lib/pronto/formatter/github_formatter.rb b/lib/pronto/formatter/github_formatter.rb index 1ab21075..2c81b961 100644 --- a/lib/pronto/formatter/github_formatter.rb +++ b/lib/pronto/formatter/github_formatter.rb @@ -3,13 +3,14 @@ module Formatter class GithubFormatter def format(messages, repo) commit_messages = messages.map do |message| - github_slug = repo.remotes.map(&:github_slug).compact.first + github_slug = repo.github_slug sha = message.commit_sha position = message.line.commit_line.position if message.line - path = message.path body = message.msg + path = message.path - create_comment(github_slug, sha, position, path, body) + comment = Github::Comment.new(github_slug, sha, body, path, position) + create_comment(github_slug, sha, comment) end "#{commit_messages.compact.count} Pronto messages posted to GitHub" @@ -17,18 +18,10 @@ def format(messages, repo) private - def create_comment(repo, sha, position, path, body) + def create_comment(repo, sha, comment) comments = client.commit_comments(repo, sha) - - existing_comment = comments.find do |comment| - comment.position == position && - comment.path == path && - comment.body == body - end - - unless existing_comment - client.create_commit_comment(repo, sha, body, path, nil, position) - end + existing = comments.any? { |c| comment == c } + client.create_commit_comment(repo, sha, comment) unless existing end def client diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb index 35c5196a..e3852438 100644 --- a/lib/pronto/git/repository.rb +++ b/lib/pronto/git/repository.rb @@ -5,8 +5,8 @@ def initialize(path) @repo = Rugged::Repository.new(path) end - def remotes - @repo.remotes.map { |remote| Remote.new(remote) } + def github_slug + remotes.map(&:github_slug).compact.first end def diff(commit) @@ -48,6 +48,10 @@ 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 f7185827..2d2e06a0 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -1,8 +1,15 @@ module Pronto class Github - extend Forwardable + def commit_comments(repo, sha) + client.commit_comments(repo, sha).map do |comment| + Comment.new(repo, sha, comment.body, comment.path, comment.body) + end + end - def_delegators :client, :commit_comments, :create_commit_comment + def create_commit_comment(repo, sha, comment) + client.create_commit_comment(repo, sha, comment.body, comment.path, + nil, comment.position) + end private @@ -13,5 +20,13 @@ def client def access_token ENV['GITHUB_ACCESS_TOKEN'] end + + class Comment < Struct.new(:repo, :sha, :body, :path, :position) + def ==(other) + position == other.position && + path == other.path && + body == other.body + end + end end end From e498de7e837d81b7d2108cbce90803fd52019a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 10 Aug 2014 20:00:10 +0300 Subject: [PATCH 23/30] Add CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..77a380ad --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,7 @@ +# Changelog + +## 0.3.0 + +### Changes + +* [#29](https://github.com/mmozuras/pronto/issues/29): Be compatible and depend on rugged '0.21.0'. From d8064e9610a571e1c7e26ee67dbf287fd2b72a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 10 Aug 2014 20:32:54 +0300 Subject: [PATCH 24/30] Performance improvement - cache comments retrieved from GitHub --- CHANGELOG.md | 6 +++++ lib/pronto/github.rb | 10 +++++-- .../formatter/github_formattter_spec.rb | 2 +- spec/pronto/github_spec.rb | 27 +++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 spec/pronto/github_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 77a380ad..ad1a2e13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## 0.3.0 +### New features + ### Changes * [#29](https://github.com/mmozuras/pronto/issues/29): Be compatible and depend on rugged '0.21.0'. +* Performance improvement - use Rugged::Blame instead of one provided by Grit. +* Performance improvement - cache comments retrieved from GitHub. + +### Bugs fixed diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb index 2d2e06a0..fc8b58f5 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -1,8 +1,14 @@ module Pronto class Github + def initialize + @comment_cache = {} + end + def commit_comments(repo, sha) - client.commit_comments(repo, sha).map do |comment| - Comment.new(repo, sha, comment.body, comment.path, comment.body) + @comment_cache["#{repo}/#{sha}"] ||= begin + client.commit_comments(repo, sha).map do |comment| + Comment.new(repo, sha, comment.body, comment.path, comment.body) + end end end diff --git a/spec/pronto/formatter/github_formattter_spec.rb b/spec/pronto/formatter/github_formattter_spec.rb index 0980295f..b6515541 100644 --- a/spec/pronto/formatter/github_formattter_spec.rb +++ b/spec/pronto/formatter/github_formattter_spec.rb @@ -16,7 +16,7 @@ module Formatter specify do Octokit::Client.any_instance .should_receive(:commit_comments) - .twice + .once .and_return([]) Octokit::Client.any_instance diff --git a/spec/pronto/github_spec.rb b/spec/pronto/github_spec.rb new file mode 100644 index 00000000..208cfc85 --- /dev/null +++ b/spec/pronto/github_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +module Pronto + describe Github do + let(:github) { Github.new } + + describe '#commit_comments' do + subject { github.commit_comments(repo, sha) } + + context 'three requests for same comments' do + let(:repo) { 'mmozuras/pronto' } + let(:sha) { '61e4bef' } + + specify do + Octokit::Client.any_instance + .should_receive(:commit_comments) + .once + .and_return([]) + + subject + subject + subject + end + end + end + end +end From d4cb55870a5a503b8a7843b2724b55b6a3dc4303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Sun, 10 Aug 2014 20:45:52 +0300 Subject: [PATCH 25/30] Add '--exit-code' option for 'pronto run' With it, pronto exits with non-zero code if there were any warnings/errors. Resolves #27 --- CHANGELOG.md | 6 ++++-- lib/pronto.rb | 4 +++- lib/pronto/cli.rb | 7 ++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad1a2e13..5db7be7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,12 @@ ### New features +* [#27](https://github.com/mmozuras/pronto/issues/27): '--exit-code' option for 'pronto run'. Pronto exits with non-zero code if there were any warnings/errors. + ### Changes * [#29](https://github.com/mmozuras/pronto/issues/29): Be compatible and depend on rugged '0.21.0'. -* Performance improvement - use Rugged::Blame instead of one provided by Grit. -* Performance improvement - cache comments retrieved from GitHub. +* Performance improvement: use Rugged::Blame instead of one provided by Grit. +* Performance improvement: cache comments retrieved from GitHub. ### Bugs fixed diff --git a/lib/pronto.rb b/lib/pronto.rb index 683486c7..16db5678 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -29,7 +29,9 @@ def self.run(commit = 'master', repo_path = '.', formatter = nil) result = run_all_runners(patches) formatter ||= default_formatter - formatter.format(result, repo) + puts formatter.format(result, repo) + + result end def self.gem_names diff --git a/lib/pronto/cli.rb b/lib/pronto/cli.rb index 2a42be93..a839d8d4 100644 --- a/lib/pronto/cli.rb +++ b/lib/pronto/cli.rb @@ -14,6 +14,10 @@ def is_thor_reserved_word?(word, type) desc 'run', 'Run Pronto' + method_option :'exit-code', + type: :boolean, + banner: 'Exits with non-zero code if there were any warnings/errors.' + method_option :commit, type: :string, default: 'master', @@ -39,7 +43,8 @@ def run end formatter = ::Pronto::Formatter.get(options[:formatter]) - puts ::Pronto.run(options[:commit], '.', formatter) + messages = ::Pronto.run(options[:commit], '.', formatter) + exit(messages.count) if options[:'exit-code'] rescue Rugged::RepositoryError puts '"pronto" should be run from a git repository' end From b2575d73f39f3a758d8b5a389e443974b24ab959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 19 Aug 2014 14:46:10 +0300 Subject: [PATCH 26/30] Extract Git::Patches#find_line from Git::Line --- lib/pronto/git/line.rb | 9 ++------- lib/pronto/git/patches.rb | 6 ++++++ spec/pronto/git/patches_spec.rb | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 spec/pronto/git/patches_spec.rb diff --git a/lib/pronto/git/line.rb b/lib/pronto/git/line.rb index e306fc47..7ac1fa0b 100644 --- a/lib/pronto/git/line.rb +++ b/lib/pronto/git/line.rb @@ -21,13 +21,8 @@ def commit_line @commit_line ||= begin patches = patch.repo.show_commit(commit_sha) - commit_patch = patches.find do |p| - patch.new_file_full_path == p.new_file_full_path - end - - lines = commit_patch ? commit_patch.lines : [] - result = lines.find { |l| blame[:orig_start_line_number] == l.new_lineno } - + result = patches.find_line(patch.new_file_full_path, + blame[:orig_start_line_number]) result || self # no commit_line means that it was just added end end diff --git a/lib/pronto/git/patches.rb b/lib/pronto/git/patches.rb index 041b6cb4..faef168d 100644 --- a/lib/pronto/git/patches.rb +++ b/lib/pronto/git/patches.rb @@ -13,6 +13,12 @@ def initialize(repo, commit, patches) def each(&block) @patches.each(&block) end + + def find_line(path, line) + patch = find { |p| p.new_file_full_path == path } + lines = patch ? patch.lines : [] + lines.find { |l| l.new_lineno == line } + end end end end diff --git a/spec/pronto/git/patches_spec.rb b/spec/pronto/git/patches_spec.rb new file mode 100644 index 00000000..8bf8c6a4 --- /dev/null +++ b/spec/pronto/git/patches_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +module Pronto + module Git + describe Patches do + describe '#find_line' do + subject { Patches.new(repo, commit, patches).find_line(path, line) } + + let(:repo) { nil } + let(:commit) { nil } + + let(:path) { '/test.rb' } + let(:line) { 1 } + + context 'no patches' do + let(:patches) { [] } + it { should be_nil } + end + end + end + end +end From 013c4c4c9689c05dab0fdcf76670d2ab30fd6114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 19 Aug 2014 19:53:27 +0300 Subject: [PATCH 27/30] Return a path instead of string from Repository#path --- lib/pronto/git/patch.rb | 5 +---- lib/pronto/git/repository.rb | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/pronto/git/patch.rb b/lib/pronto/git/patch.rb index 0c5729ee..d6f3e20f 100644 --- a/lib/pronto/git/patch.rb +++ b/lib/pronto/git/patch.rb @@ -1,5 +1,3 @@ -require 'pathname' - module Pronto module Git class Patch < Struct.new(:patch, :repo) @@ -36,8 +34,7 @@ def deleted_lines end def new_file_full_path - repo_path = Pathname.new(repo.path).parent - repo_path.join(delta.new_file[:path]) + repo.path.join(delta.new_file[:path]) end end end diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb index e3852438..9f850670 100644 --- a/lib/pronto/git/repository.rb +++ b/lib/pronto/git/repository.rb @@ -1,3 +1,5 @@ +require 'pathname' + module Pronto module Git class Repository @@ -29,7 +31,7 @@ def show_commit(sha) end def path - @repo.path + Pathname.new(@repo.path).parent end def blame(patch, lineno) From d10596b781d7ad1a50ee663166405c674066a016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 19 Aug 2014 20:10:04 +0300 Subject: [PATCH 28/30] Formatter for GitHub pull requests Closes #16 --- CHANGELOG.md | 1 + lib/pronto.rb | 1 + lib/pronto/formatter/formatter.rb | 1 + lib/pronto/formatter/github_formatter.rb | 2 +- .../github_pull_request_formatter.rb | 41 +++++++++++++++++++ lib/pronto/git/repository.rb | 9 ++++ lib/pronto/github.rb | 17 ++++++++ lib/pronto/message.rb | 4 ++ spec/pronto/formatter/formatter_spec.rb | 7 +++- spec/pronto/git/patch_spec.rb | 2 +- 10 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 lib/pronto/formatter/github_pull_request_formatter.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5db7be7d..596349a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New features * [#27](https://github.com/mmozuras/pronto/issues/27): '--exit-code' option for 'pronto run'. Pronto exits with non-zero code if there were any warnings/errors. +* [#16](https://github.com/mmozuras/pronto/issues/16): New formatter: GithubPullRequestFormatter. Writes review comments on GitHub pull requests. ### Changes diff --git a/lib/pronto.rb b/lib/pronto.rb index 16db5678..9d7524ab 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -16,6 +16,7 @@ require 'pronto/formatter/text_formatter' require 'pronto/formatter/json_formatter' require 'pronto/formatter/github_formatter' +require 'pronto/formatter/github_pull_request_formatter' require 'pronto/formatter/checkstyle_formatter' require 'pronto/formatter/formatter' diff --git a/lib/pronto/formatter/formatter.rb b/lib/pronto/formatter/formatter.rb index ba679bd1..db58f831 100644 --- a/lib/pronto/formatter/formatter.rb +++ b/lib/pronto/formatter/formatter.rb @@ -11,6 +11,7 @@ def self.names FORMATTERS = { 'github' => GithubFormatter, + 'github_pr' => GithubPullRequestFormatter, 'json' => JsonFormatter, 'checkstyle' => CheckstyleFormatter, 'text' => TextFormatter diff --git a/lib/pronto/formatter/github_formatter.rb b/lib/pronto/formatter/github_formatter.rb index 2c81b961..f7655c84 100644 --- a/lib/pronto/formatter/github_formatter.rb +++ b/lib/pronto/formatter/github_formatter.rb @@ -5,9 +5,9 @@ def format(messages, repo) commit_messages = messages.map do |message| github_slug = repo.github_slug sha = message.commit_sha - position = message.line.commit_line.position if message.line body = message.msg path = message.path + position = message.line.commit_line.position if message.line comment = Github::Comment.new(github_slug, sha, body, path, position) create_comment(github_slug, sha, comment) diff --git a/lib/pronto/formatter/github_pull_request_formatter.rb b/lib/pronto/formatter/github_pull_request_formatter.rb new file mode 100644 index 00000000..270673f1 --- /dev/null +++ b/lib/pronto/formatter/github_pull_request_formatter.rb @@ -0,0 +1,41 @@ +module Pronto + module Formatter + class GithubPullRequestFormatter + def format(messages, repo) + commit_messages = messages.map do |message| + github_slug = repo.github_slug + body = message.msg + path = message.path + + commits = repo.commits_until(message.commit_sha) + + line = nil + sha = commits.find do |commit| + patches = repo.show_commit(commit) + line = patches.find_line(message.full_path, message.line.new_lineno) + line + end + + position = line.position - 1 + + comment = Github::Comment.new(github_slug, sha, body, path, position) + create_comment(github_slug, sha, comment) + end + + "#{commit_messages.compact.count} Pronto messages posted to GitHub" + end + + private + + def create_comment(repo, sha, comment) + comments = client.pull_comments(repo, sha) + existing = comments.any? { |c| comment == c } + client.create_pull_comment(repo, sha, comment) unless existing + end + + def client + @client ||= Github.new + end + end + end +end diff --git a/lib/pronto/git/repository.rb b/lib/pronto/git/repository.rb index 9f850670..24bd4b16 100644 --- a/lib/pronto/git/repository.rb +++ b/lib/pronto/git/repository.rb @@ -30,6 +30,15 @@ def show_commit(sha) Patches.new(self, sha, diff.patches) end + def commits_until(sha) + result = [] + @repo.walk('HEAD', Rugged::SORT_TOPO).take_while do |commit| + result << commit.oid + !commit.oid.start_with?(sha) + end + result + end + def path Pathname.new(@repo.path).parent end diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb index fc8b58f5..7da4d5d6 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -4,6 +4,14 @@ def initialize @comment_cache = {} end + def pull_comments(repo, sha) + @comment_cache["#{repo}/#{pull_id}/#{sha}"] ||= begin + client.pull_comments(repo, pull_id).map do |comment| + Comment.new(repo, sha, comment.body, comment.path, comment.body) + end + end + end + def commit_comments(repo, sha) @comment_cache["#{repo}/#{sha}"] ||= begin client.commit_comments(repo, sha).map do |comment| @@ -17,12 +25,21 @@ def create_commit_comment(repo, sha, comment) nil, comment.position) end + def create_pull_comment(repo, sha, comment) + client.create_pull_comment(repo, pull_id, comment.body, sha, comment.path, + comment.position) + end + private def client @client ||= Octokit::Client.new(access_token: access_token) end + def pull_id + ENV['PULL_REQUEST_ID'].to_i + end + def access_token ENV['GITHUB_ACCESS_TOKEN'] end diff --git a/lib/pronto/message.rb b/lib/pronto/message.rb index a8fcafa6..85918575 100644 --- a/lib/pronto/message.rb +++ b/lib/pronto/message.rb @@ -17,6 +17,10 @@ def initialize(path, line, level, msg, commit_sha = nil) @commit_sha ||= line.commit_sha if line end + def full_path + repo.path.join(path) if repo + end + def repo line.patch.repo if line end diff --git a/spec/pronto/formatter/formatter_spec.rb b/spec/pronto/formatter/formatter_spec.rb index 43372e61..2f357406 100644 --- a/spec/pronto/formatter/formatter_spec.rb +++ b/spec/pronto/formatter/formatter_spec.rb @@ -10,6 +10,11 @@ module Formatter it { should be_an_instance_of GithubFormatter } end + context 'github_pr' do + let(:name) { 'github_pr' } + it { should be_an_instance_of GithubPullRequestFormatter } + end + context 'json' do let(:name) { 'json' } it { should be_an_instance_of JsonFormatter } @@ -38,7 +43,7 @@ module Formatter describe '.names' do subject { Formatter.names } - it { should =~ %w(github json checkstyle text) } + it { should =~ %w(github github_pr json checkstyle text) } end end end diff --git a/spec/pronto/git/patch_spec.rb b/spec/pronto/git/patch_spec.rb index e045fb96..8775f55c 100644 --- a/spec/pronto/git/patch_spec.rb +++ b/spec/pronto/git/patch_spec.rb @@ -35,7 +35,7 @@ module Git let(:rugged_patch) do double(delta: double(new_file: { path: 'test.md' })) end - let(:repo) { double(path: '/house/of/cards/orig.md') } + let(:repo) { double(path: Pathname.new('/house/of/cards')) } its(:to_s) { should == '/house/of/cards/test.md' } end end From b74d51c34224f5547486d3c4cb4c16a18cafe51b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 19 Aug 2014 20:14:26 +0300 Subject: [PATCH 29/30] Fix: pass position when creating Comment from GitHub responses --- lib/pronto/github.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb index 7da4d5d6..bff2d16c 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -7,7 +7,7 @@ def initialize def pull_comments(repo, sha) @comment_cache["#{repo}/#{pull_id}/#{sha}"] ||= begin client.pull_comments(repo, pull_id).map do |comment| - Comment.new(repo, sha, comment.body, comment.path, comment.body) + Comment.new(repo, sha, comment.body, comment.path, comment.position) end end end @@ -15,7 +15,7 @@ def pull_comments(repo, sha) def commit_comments(repo, sha) @comment_cache["#{repo}/#{sha}"] ||= begin client.commit_comments(repo, sha).map do |comment| - Comment.new(repo, sha, comment.body, comment.path, comment.body) + Comment.new(repo, sha, comment.body, comment.path, comment.position) end end end From 122c62ae573211f1e35fc553798731ea610b15e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mindaugas=20Moz=C5=ABras?= Date: Tue, 19 Aug 2014 20:27:49 +0300 Subject: [PATCH 30/30] Bump version to 0.3.0 --- lib/pronto/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pronto/version.rb b/lib/pronto/version.rb index 0beca8ec..459ca61f 100644 --- a/lib/pronto/version.rb +++ b/lib/pronto/version.rb @@ -1,3 +1,3 @@ module Pronto - VERSION = '0.2.6' + VERSION = '0.3.0' end