Pull request formatter: Find position in full diff and fix commit_id

lib/pronto.rb

@@ -34,7 +34,7 @@ def self.run(commit = 'master', repo_path = '.',
 
     result = run_all_runners(patches)
 
-    formatted = formatter.format(result, repo)
+    formatted = formatter.format(result, repo, patches)
     puts formatted if formatted
 
     result

lib/pronto/formatter/checkstyle_formatter.rb

@@ -7,7 +7,7 @@ def initialize
         @output = ''
       end
 
-      def format(messages, _)
+      def format(messages, _, _)
         open_xml
         process_messages(messages)
         close_xml

lib/pronto/formatter/github_formatter.rb

@@ -1,7 +1,7 @@
 module Pronto
   module Formatter
     class GithubFormatter
-      def format(messages, repo)
+      def format(messages, repo, _)
         messages = messages.uniq { |message| [message.msg, message.line.new_lineno] }
         client = Github.new(repo)
 

lib/pronto/formatter/github_pull_request_formatter.rb

@@ -1,24 +1,17 @@
 module Pronto
   module Formatter
     class GithubPullRequestFormatter
-      def format(messages, repo)
+      def format(messages, repo, patches)
         messages = messages.uniq { |message| [message.msg, message.line.new_lineno] }
         client = Github.new(repo)
+        head = repo.head_commit_sha
 
         commit_messages = messages.map do |message|
           body = message.msg
           path = message.path
+          line = patches.find_line(message.full_path, message.line.new_lineno)
 
-          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
-
-          create_comment(client, sha, body, path, line.position)
+          create_comment(client, head, body, path, line.position)
         end
 
         "#{commit_messages.compact.count} Pronto messages posted to GitHub"

lib/pronto/formatter/gitlab_formatter.rb

@@ -1,7 +1,7 @@
 module Pronto
   module Formatter
     class GitlabFormatter
-      def format(messages, repo)
+      def format(messages, repo, _)
         messages = messages.uniq { |message| [message.msg, message.line.new_lineno] }
         client = Gitlab.new repo
 

lib/pronto/formatter/json_formatter.rb

@@ -3,7 +3,7 @@
 module Pronto
   module Formatter
     class JsonFormatter
-      def format(messages, _)
+      def format(messages, _, _)
         messages.map do |message|
           lineno = message.line.new_lineno if message.line
 

lib/pronto/formatter/null_formatter.rb

@@ -1,7 +1,7 @@
 module Pronto
   module Formatter
     class NullFormatter
-      def format(_, _)
+      def format(_, _, _)
       end
     end
   end

lib/pronto/formatter/text_formatter.rb

@@ -1,7 +1,7 @@
 module Pronto
   module Formatter
     class TextFormatter
-      def format(messages, _)
+      def format(messages, _, _)
         messages.map do |message|
           level = message.level[0].upcase
           line = message.line

lib/pronto/git/repository.rb

@@ -58,6 +58,10 @@ def remote_urls
         @repo.remotes.map(&:url)
       end
 
+      def head_commit_sha
+        head.oid
+      end
+
       private
 
       def empty_patches(sha)

spec/pronto/formatter/checkstyle_formatter_spec.rb

@@ -6,7 +6,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, nil) }
+        subject { formatter.format(messages, nil, nil) }
         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') }

spec/pronto/formatter/github_formattter_spec.rb

@@ -6,7 +6,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, repo) }
+        subject { formatter.format(messages, repo, nil) }
         let(:messages) { [message, message] }
         let(:repo) { Git::Repository.new('spec/fixtures/test.git') }
         let(:message) { Message.new('path/to', line, :warning, 'crucial') }
@@ -28,7 +28,7 @@ module Formatter
       end
 
       describe '#format without duplicates' do
-        subject { formatter.format(messages, repo) }
+        subject { formatter.format(messages, repo, nil) }
         let(:messages) { [message1, message2] }
         let(:repo) { Git::Repository.new('spec/fixtures/test.git') }
         let(:message1) { Message.new('path/to1', line1, :warning, 'crucial') }

spec/pronto/formatter/github_pull_request_formattter_spec.rb

@@ -8,11 +8,12 @@ module Formatter
       let(:repo) { Git::Repository.new('spec/fixtures/test.git') }
 
       describe '#format' do
-        subject { formatter.format(messages, repo) }
+        subject { formatter.format(messages, repo, patches) }
         let(:messages) { [message, message] }
         let(:message) { Message.new(patch.new_file_full_path, line, :warning, 'crucial') }
         let(:patch) { repo.show_commit('64dadfd').first }
         let(:line) { patch.added_lines.first }
+        let(:patches) { repo.diff('64dadfd^') }
 
         specify do
           ENV['PULL_REQUEST_ID'] = '10'

spec/pronto/formatter/gitlab_formatter_spec.rb

@@ -9,7 +9,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, repo) }
+        subject { formatter.format(messages, repo, nil) }
         let(:messages) { [message, message] }
         let(:repo) { Git::Repository.new('spec/fixtures/test.git') }
         let(:message) { Message.new('path/to', line, :warning, 'crucial') }
@@ -31,7 +31,7 @@ module Formatter
       end
 
       describe '#format without duplicates' do
-        subject { formatter.format(messages, repo) }
+        subject { formatter.format(messages, repo, nil) }
         let(:messages) { [message1, message2] }
         let(:repo) { Git::Repository.new('spec/fixtures/test.git') }
         let(:message1) { Message.new('path/to1', line1, :warning, 'crucial') }

spec/pronto/formatter/json_formatter_spec.rb

@@ -6,7 +6,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, nil) }
+        subject { formatter.format(messages, nil, nil) }
         let(:messages) { [message, message] }
         let(:message) { Message.new('path/to', line, :warning, 'crucial') }
         let(:line) { double(new_lineno: 1, commit_sha: nil) }

spec/pronto/formatter/null_formatter_spec.rb

@@ -6,7 +6,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, nil) }
+        subject { formatter.format(messages, nil, nil) }
         let(:messages) { [message, message] }
         let(:message) { Message.new('path/to', line, :warning, 'crucial') }
         let(:line) { double(new_lineno: 1, commit_sha: '123') }

spec/pronto/formatter/text_formatter_spec.rb

@@ -6,7 +6,7 @@ module Formatter
       let(:formatter) { described_class.new }
 
       describe '#format' do
-        subject { formatter.format(messages, nil) }
+        subject { formatter.format(messages, nil, nil) }
         let(:messages) { [message, message] }
         let(:message) { Message.new('path/to', line, :warning, 'crucial') }
         let(:line) { double(new_lineno: 1, commit_sha: '123') }