Skip to content

Commit

Permalink
Add --clean option to the style checker
Browse files Browse the repository at this point in the history
Closes #8
  • Loading branch information
Emanuel Evans committed May 3, 2016
1 parent 2bb402a commit 44a2345
Show file tree
Hide file tree
Showing 11 changed files with 169 additions and 17 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ It probably makes sense to put `style-check` in either the `pre` or
- `GITHUB_ACCESS_TOKEN`: A Github API auth token for a user with commit
access to your repo. (Can also be set with the `-g` option.)

`style-check` also has the following option:

- `--clean`: Cleans old comments from the pull request before commenting again,
to avoid double-commenting for the same issue. *Warning: only use this option
if you have a special Github user for Circlemator, otherwise you may end up
accidentally deleting user code review comments!*

### Self-merge release branch

Preamble: at Rainforest, our process for getting code into production
Expand Down
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
test:
pre:
- bundle exec exe/circlemator cancel-old
- bundle exec exe/circlemator style-check --base-branch=master
- bundle exec exe/circlemator style-check --base-branch=master --clean
4 changes: 4 additions & 0 deletions exe/circlemator
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ parser = OptionParser.new do |opts|
options[:circle_api_token] = t
end

opts.on('--clean', 'Clean existing comments before commenting on a pull request') do
options[:clean] = true
end

opts.on('-h', '--help', 'Print this help message') do
puts opts
exit
Expand Down
22 changes: 18 additions & 4 deletions lib/circlemator/github_repo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,24 @@ def initialize(url, repo)
end
end

base_uri 'https://api.github.com/repos'
class BadResponseError < StandardError
def initialize(response)
super "Bad response from github: #{response.inspect}"
end
end

base_uri 'https://api.github.com'

def initialize(user:, repo:, github_auth_token:, **_opts)
@user = user
@repo = repo
@auth_token = github_auth_token
end

def get_current_user
self.class.get('/user', opts.merge(basic_auth: auth))
end

def get(path, opts = {})
self.class.get(fix_path(path), opts.merge(basic_auth: auth))
end
Expand All @@ -33,16 +43,20 @@ def put(path, opts = {})
self.class.put(fix_path(path), opts.merge(basic_auth: auth))
end

def delete(path, opts = {})
self.class.delete(fix_path(path), opts.merge(basic_auth: auth))
end

private

def fix_path(path)
case path
when %r(\A#{self.class.base_uri}(/#{@user}/#{@repo}.*)\z)
when %r(\A#{self.class.base_uri}(/repos/#{@user}/#{@repo}.*)\z)
$1
when %r(\A#{self.class.base_uri})
when %r(\A#{self.class.base_uri}/repos)
raise WrongRepo.new(path, "#{@user}/#{@repo}")
when %r(\A/.*)
"/#{@user}/#{@repo}#{path}"
"/repos/#{@user}/#{@repo}#{path}"
else
raise InvalidPath, path
end
Expand Down
36 changes: 36 additions & 0 deletions lib/circlemator/pr_cleaner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# frozen_string_literal: true
require 'circlemator/github_repo'

module Circlemator
class PrCleaner
def initialize(github_repo:, pr_url:)
@github_repo = github_repo
@pr_url = pr_url
get_user_id
end

def clean!
response = @github_repo.get("#{@pr_url}/comments")
if response.code != 200
raise ::Circlemator::GithubRepo::BadResponseError, response
end

comments = JSON.parse(response.body)
comments.each do |comment|
if comment.dig('user', 'id') == @user_id
@github_repo.delete("#{@pr_url}/comments/#{comment.fetch('id')}")
end
end
end

private

def get_user_id
response = @github_repo.get_current_user
if response.code != 200
raise ::Circlemator::GithubRepo::BadResponseError, response
end
@user_id = JSON.parse(response.body).fetch('id')
end
end
end
8 changes: 1 addition & 7 deletions lib/circlemator/pr_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@

module Circlemator
class PrFinder
class BadResponseError < StandardError
def initialize(response)
super "Bad response from github: #{response.inspect}"
end
end

def initialize(github_repo:, base_branch:, compare_branch:, sha:, **_opts)
@github_repo = github_repo
@base_branch = base_branch
Expand All @@ -19,7 +13,7 @@ def initialize(github_repo:, base_branch:, compare_branch:, sha:, **_opts)
def find_pr
response = @github_repo.get '/pulls', query: { base: @base_branch }
if response.code != 200
raise BadResponseError, response
raise ::Circlemator::GithubRepo::BadResponseError, response
end

prs = JSON.parse(response.body)
Expand Down
6 changes: 5 additions & 1 deletion lib/circlemator/style_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'pronto/rubocop'
require 'pronto/commentator'
require 'circlemator/pr_finder'
require 'circlemator/pr_cleaner'

module Circlemator
class StyleChecker
Expand All @@ -14,8 +15,11 @@ def initialize(opts)
end

def check!
pr_number, _ = PrFinder.new(@opts).find_pr
pr_number, pr_url = PrFinder.new(@opts).find_pr
if pr_number
if @opts[:clean]
PrCleaner.new(github_repo: @opts.fetch(:github_repo), pr_url: pr_url).clean!
end
ENV['PULL_REQUEST_ID'] = pr_number.to_s
formatter = ::Pronto::Formatter::GithubPullRequestFormatter.new
else
Expand Down
6 changes: 3 additions & 3 deletions spec/circlemator/github_repo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
context 'with a path' do
it 'updates the path and adds the authorization' do
expect(described_class)
.to receive(:get).with("/#{user}/#{repo}/pulls",
.to receive(:get).with("/repos/#{user}/#{repo}/pulls",
query: { foo: :bar },
basic_auth: { username: auth_token, password: 'x-oauth-basic' })

Expand All @@ -22,7 +22,7 @@
context 'with a full URL' do
it 'changes the URL to a path' do
expect(described_class)
.to receive(:get).with("/#{user}/#{repo}/pulls", any_args)
.to receive(:get).with("/repos/#{user}/#{repo}/pulls", any_args)

github_repo.get("https://api.github.com/repos/#{user}/#{repo}/pulls")
end
Expand All @@ -46,7 +46,7 @@
describe '#put' do
it 'updates the path and adds the authorization' do
expect(described_class)
.to receive(:put).with("/#{user}/#{repo}/pulls/123/merge",
.to receive(:put).with("/repos/#{user}/#{repo}/pulls/123/merge",
body: { foo: :bar },
basic_auth: { username: auth_token, password: 'x-oauth-basic' })

Expand Down
75 changes: 75 additions & 0 deletions spec/circlemator/pr_cleaner_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# frozen_string_literal: true
require 'circlemator/pr_cleaner'
require 'circlemator/github_repo'
require 'json'

RSpec.describe Circlemator::PrCleaner do
describe '#clean!' do
let(:github_repo) do
Circlemator::GithubRepo.new(user: 'octocat', repo: 'Hello-World', github_auth_token: 'abc123')
end
let(:pr_url) { 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' }
let(:cleaner) do
described_class.new github_repo: github_repo,
pr_url: pr_url
end
let(:user_id) { 7 }
let(:user_response) do
{
'login' => 'octocat',
'id' => user_id,
}.to_json
end
let(:comments_response) do
[
{
'id' => 1,
'user' => {
'id' => user_id,
},
},
{
'id' => 2,
'user' => {
'id' => user_id + 1,
},
},
{
'id' => 3,
'user' => {
'id' => user_id,
},
},
].to_json
end

subject { cleaner.clean! }

before do
allow(github_repo)
.to receive(:get_current_user).and_return double(code: 200, body: user_response)
allow(github_repo)
.to receive(:get).with("#{pr_url}/comments").and_return double(code: 200, body: comments_response)
end

context 'with a successful response' do
it 'deletes all the comments for the PR for the authenticated user' do
expect(github_repo).to receive(:delete).with(%r(comments/1)).once
expect(github_repo).to receive(:delete).with(%r(comments/3)).once
expect(github_repo).to_not receive(:delete).with(%r(comments/2))

subject
end
end

context 'with an unsuccessful response' do
before do
allow(github_repo).to receive(:get).and_return double(code: 500, body: 'BAD')
end

it 'raises an error' do
expect { subject }.to raise_error Circlemator::GithubRepo::BadResponseError
end
end
end
end
2 changes: 1 addition & 1 deletion spec/circlemator/pr_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
end

it 'raises an error' do
expect { subject }.to raise_error Circlemator::PrFinder::BadResponseError
expect { subject }.to raise_error Circlemator::GithubRepo::BadResponseError
end
end

Expand Down
18 changes: 18 additions & 0 deletions spec/circlemator/style_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,24 @@

expect(ENV['PULL_REQUEST_ID']).to eq pr_number.to_s
end

context 'with the --clean option' do
let(:checker) do
Circlemator::StyleChecker.new github_repo: github_repo,
sha: 'abc123',
base_branch: 'master',
compare_branch: 'topic',
clean: true
end

it 'cleans existing comments before running pronto' do
allow(Pronto::Formatter::GithubPullRequestFormatter).to receive(:new)
allow(Pronto).to receive(:run)
expect(Circlemator::PrCleaner).to receive(:new).and_return double(clean!: nil)

subject
end
end
end

context 'without an open PR' do
Expand Down

0 comments on commit 44a2345

Please sign in to comment.