Skip to content

Commit 4713783

Browse files
piotrmurachpiotrmurach
committed
Change pull requests comments to remove params checks.
1 parent 8936561 commit 4713783

8 files changed

Lines changed: 64 additions & 91 deletions

File tree

‎lib/github_api/client/pull_requests/comments.rb‎

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,6 @@
22

33
module Github
44
class Client::PullRequests::Comments < API
5-
6-
VALID_REQUEST_COM_PARAM_NAMES = %w[
7-
body
8-
commit_id
9-
path
10-
position
11-
in_reply_to
12-
mime_type
13-
resource
14-
].freeze
15-
165
# List comments on a pull request
176
#
187
# @example
@@ -51,7 +40,7 @@ def list(*args)
5140
return response unless block_given?
5241
response.each { |el| yield el }
5342
end
54-
alias :all :list
43+
alias_method :all, :list
5544

5645
# Get a single comment for pull requests
5746
#
@@ -71,7 +60,7 @@ def get(*args)
7160

7261
get_request("/repos/#{arguments.user}/#{arguments.repo}/pulls/comments/#{arguments.number}", arguments.params)
7362
end
74-
alias :find :get
63+
alias_method :find, :get
7564

7665
# Create a pull request comment
7766
#
@@ -110,9 +99,7 @@ def get(*args)
11099
#
111100
# @api public
112101
def create(*args)
113-
arguments(args, required: [:user, :repo, :number]) do
114-
permit VALID_REQUEST_COM_PARAM_NAMES
115-
end
102+
arguments(args, required: [:user, :repo, :number])
116103

117104
post_request("/repos/#{arguments.user}/#{arguments.repo}/pulls/#{arguments.number}/comments", arguments.params)
118105
end
@@ -130,9 +117,7 @@ def create(*args)
130117
#
131118
# @api public
132119
def edit(*args)
133-
arguments(args, required: [:user, :repo, :number]) do
134-
permit VALID_REQUEST_COM_PARAM_NAMES
135-
end
120+
arguments(args, required: [:user, :repo, :number])
136121

137122
patch_request("/repos/#{arguments.user}/#{arguments.repo}/pulls/comments/#{arguments.number}", arguments.params)
138123
end
@@ -149,17 +134,5 @@ def delete(*args)
149134

150135
delete_request("/repos/#{arguments.user}/#{arguments.repo}/pulls/comments/#{arguments.number}", arguments.params)
151136
end
152-
153-
private
154-
155-
# To let user know that the params supplied are wrong before request is made
156-
def _validate_reply_to(params)
157-
if params['in_reply_to'] && !assert_required_keys(%w[ body in_reply_to ], params)
158-
raise ArgumentError, "Required params are: #{%w[ body in_reply_to].join(',')}"
159-
160-
elsif !assert_required_keys(VALID_REQUEST_COM_PARAM_NAMES - %w[ in_reply_to ], params)
161-
raise ArgumentError, "Required params are: #{VALID_REQUEST_COM_PARAM_NAMES.join(', ')}"
162-
end
163-
end
164137
end # PullRequests::Comments
165138
end # Github

‎spec/github/client/pull_requests/comments/create_spec.rb‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments, '#create' do
5+
RSpec.describe Github::Client::PullRequests::Comments, '#create' do
66
let(:user) { 'peter-murach' }
77
let(:repo) { 'github' }
88
let(:request_path) { "/repos/#{user}/#{repo}/pulls/#{number}/comments" }
@@ -20,8 +20,8 @@
2020

2121
before {
2222
stub_post(request_path).with(inputs.except('unrelated')).
23-
to_return(:body => body, :status => status,
24-
:headers => {:content_type => "application/json; charset=utf-8"})
23+
to_return(body: body, status: status,
24+
headers: {content_type: "application/json; charset=utf-8"})
2525
}
2626

2727
after { reset_authentication_for(subject) }
@@ -36,19 +36,19 @@
3636
expect { subject.create user, repo }.to raise_error(ArgumentError)
3737
end
3838

39-
it "should create resource successfully" do
39+
it "creates resource successfully" do
4040
subject.create user, repo, number, inputs
41-
a_post(request_path).with(inputs).should have_been_made
41+
expect(a_post(request_path).with(inputs)).to have_been_made
4242
end
4343

44-
it "should return the resource" do
44+
it "returns the resource" do
4545
pull_request = subject.create user, repo, number, inputs
46-
pull_request.should be_a Github::ResponseWrapper
46+
expect(pull_request).to be_a(Github::ResponseWrapper)
4747
end
4848

49-
it "should get the request information" do
49+
it "gets the request information" do
5050
pull_request = subject.create user, repo, number, inputs
51-
pull_request.id.should == number
51+
expect(pull_request.id).to eq(number)
5252
end
5353
end
5454

‎spec/github/client/pull_requests/comments/delete_spec.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments, '#delete' do
5+
RSpec.describe Github::Client::PullRequests::Comments, '#delete' do
66
let(:user) { 'peter-murach' }
77
let(:repo) { 'github' }
88
let(:number) { 1 }
@@ -12,24 +12,24 @@
1212
let(:body) { fixture('pull_requests/comment.json') }
1313

1414
before {
15-
stub_delete(request_path).to_return(:body => body, :status => status,
16-
:headers => {:content_type => "application/json; charset=utf-8"})
15+
stub_delete(request_path).to_return(body: body, status: status,
16+
headers: {content_type: "application/json; charset=utf-8"})
1717
}
1818

1919
after { reset_authentication_for(subject) }
2020

2121
context 'resource removed' do
22-
it 'should raise error if number not present' do
23-
expect { subject.delete user, repo }.to raise_error(ArgumentError)
22+
it 'raises error if number not present' do
23+
expect { subject.delete(user, repo) }.to raise_error(ArgumentError)
2424
end
2525

26-
it "should remove resource successfully" do
26+
it "removes resource successfully" do
2727
subject.delete user, repo, number
28-
a_delete(request_path).should have_been_made
28+
expect(a_delete(request_path)).to have_been_made
2929
end
3030
end
3131

3232
it_should_behave_like 'request failure' do
33-
let(:requestable) { subject.delete user, repo, number }
33+
let(:requestable) { subject.delete(user, repo, number) }
3434
end
3535
end # delete

‎spec/github/client/pull_requests/comments/edit_spec.rb‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments, '#edit' do
5+
RSpec.describe Github::Client::PullRequests::Comments, '#edit' do
66
let(:user) { 'peter-murach' }
77
let(:repo) { 'github' }
88
let(:request_path) { "/repos/#{user}/#{repo}/pulls/comments/#{number}" }
@@ -16,8 +16,8 @@
1616

1717
before {
1818
stub_patch(request_path).with(inputs.except('unrelated')).
19-
to_return(:body => body, :status => status,
20-
:headers => {:content_type => "application/json; charset=utf-8"})
19+
to_return(body: body, status: status,
20+
headers: {content_type: "application/json; charset=utf-8"})
2121
}
2222

2323
after { reset_authentication_for(subject) }
@@ -28,23 +28,23 @@
2828

2929
it { expect { subject.edit }.to raise_error(ArgumentError) }
3030

31-
it "should edit resource successfully" do
32-
subject.edit user, repo, number, inputs
33-
a_patch(request_path).with(inputs).should have_been_made
31+
it "edits resource successfully" do
32+
subject.edit(user, repo, number, inputs)
33+
expect(a_patch(request_path).with(inputs)).to have_been_made
3434
end
3535

36-
it "should return the resource" do
37-
comment = subject.edit user, repo, number, inputs
38-
comment.should be_a Github::ResponseWrapper
36+
it "returns the resource" do
37+
comment = subject.edit(user, repo, number, inputs)
38+
expect(comment).to be_a(Github::ResponseWrapper)
3939
end
4040

4141
it "should get the comment information" do
42-
comment = subject.edit user, repo, number, inputs
43-
comment.user.login.should == 'octocat'
42+
comment = subject.edit(user, repo, number, inputs)
43+
expect(comment.user.login).to eq('octocat')
4444
end
4545
end
4646

4747
it_should_behave_like 'request failure' do
48-
let(:requestable) { subject.edit user, repo, number, inputs }
48+
let(:requestable) { subject.edit(user, repo, number, inputs) }
4949
end
5050
end # edit

‎spec/github/client/pull_requests/comments/get_spec.rb‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments, '#get' do
5+
RSpec.describe Github::Client::PullRequests::Comments, '#get' do
66
let(:user) { 'peter-murach' }
77
let(:repo) { 'github' }
88
let(:pull_request_id) { 1 }
99
let(:number) { 1 }
1010
let(:request_path) { "/repos/#{user}/#{repo}/pulls/comments/#{number}" }
1111

1212
before {
13-
stub_get(request_path).to_return(:body => body, :status => status,
14-
:headers => {:content_type => "application/json; charset=utf-8"})
13+
stub_get(request_path).to_return(body: body, status: status,
14+
headers: {content_type: "application/json; charset=utf-8"})
1515
}
1616

1717
after { reset_authentication_for(subject) }
@@ -24,24 +24,24 @@
2424

2525
it { expect { subject.get }.to raise_error(ArgumentError) }
2626

27-
it "should fail to get resource without comment id" do
27+
it "fails to get resource without comment id" do
2828
expect { subject.get user, repo }.to raise_error(ArgumentError)
2929
end
3030

31-
it "should get the resource" do
31+
it "gets the resource" do
3232
subject.get user, repo, pull_request_id
33-
a_get(request_path).should have_been_made
33+
expect(a_get(request_path)).to have_been_made
3434
end
3535

36-
it "should get comment information" do
36+
it "gets comment information" do
3737
comment = subject.get user, repo, number
38-
comment.id.should eq number
39-
comment.user.login.should == 'octocat'
38+
expect(comment.id).to eq number
39+
expect(comment.user.login).to eq('octocat')
4040
end
4141

42-
it "should return mash" do
42+
it "returns response wrapper" do
4343
comment = subject.get user, repo, number
44-
comment.should be_a Github::ResponseWrapper
44+
expect(comment).to be_a(Github::ResponseWrapper)
4545
end
4646
end
4747

‎spec/github/client/pull_requests/comments/list_spec.rb‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments, '#list' do
5+
RSpec.describe Github::Client::PullRequests::Comments, '#list' do
66
let(:repo) { 'github' }
77
let(:user) { 'peter-murach' }
88
let(:number) { 1 }
@@ -27,24 +27,24 @@
2727
expect { subject.list user }.to raise_error(ArgumentError)
2828
end
2929

30-
it "should get the resources" do
30+
it "gets the resources" do
3131
subject.list user, repo, number: number
32-
a_get(request_path).should have_been_made
32+
expect(a_get(request_path)).to have_been_made
3333
end
3434

3535
it_should_behave_like 'an array of resources' do
3636
let(:requestable) { subject.list user, repo, number: number }
3737
end
3838

39-
it "should get pull request comment information" do
39+
it "gets pull request comment information" do
4040
comments = subject.list user, repo, number: number
41-
comments.first.id.should == number
41+
expect(comments.first.id).to eq(number)
4242
end
4343

44-
it "should yield to a block" do
44+
it "yields to a block" do
4545
yielded = []
4646
result = subject.list(user, repo, number: number) {|obj| yielded << obj }
47-
yielded.should == result
47+
expect(yielded).to eq(result)
4848
end
4949
end
5050

@@ -53,20 +53,20 @@
5353
let(:body) { fixture('pull_requests/comments.json') }
5454
let(:status) { 200 }
5555

56-
it "should get the resources" do
56+
it "gets the resources" do
5757
subject.list user, repo
58-
a_get(request_path).should have_been_made
58+
expect(a_get(request_path)).to have_been_made
5959
end
6060

61-
it "should get pull request comment information" do
61+
it "gets pull request comment information" do
6262
comments = subject.list user, repo
63-
comments.first.id.should == number
63+
expect(comments.first.id).to eq(number)
6464
end
6565

66-
it "should yield to a block" do
66+
it "yields to a block" do
6767
yielded = []
6868
result = subject.list(user, repo) { |obj| yielded << obj }
69-
yielded.should == result
69+
expect(yielded).to eq(result)
7070
end
7171

7272
it_should_behave_like 'request failure' do

‎spec/github/client/pull_requests/comments_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests::Comments do
5+
RSpec.describe Github::Client::PullRequests::Comments do
66

77
after { reset_authentication_for subject }
88

‎spec/github/client/pull_requests/commits_spec.rb‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
require 'spec_helper'
44

5-
describe Github::Client::PullRequests, '#commits' do
5+
RSpec.describe Github::Client::PullRequests, '#commits' do
66
let(:repo) { 'github' }
77
let(:user) { 'peter-murach' }
88
let(:number) { 1 }
99
let(:request_path) { "/repos/#{user}/#{repo}/pulls/#{number}/commits" }
1010

1111
before {
12-
stub_get(request_path).to_return(:body => body, :status => status,
13-
:headers => {:content_type => "application/json; charset=utf-8"})
12+
stub_get(request_path).to_return(body: body, status: status,
13+
headers: {content_type: "application/json; charset=utf-8"})
1414
}
1515

1616
after { reset_authentication_for(subject) }
@@ -25,7 +25,7 @@
2525

2626
it "should get the resources" do
2727
subject.commits user, repo, number
28-
a_get(request_path).should have_been_made
28+
expect(a_get(request_path)).to have_been_made
2929
end
3030

3131
it_should_behave_like 'an array of resources' do
@@ -34,13 +34,13 @@
3434

3535
it "should get pull request information" do
3636
pull_requests = subject.commits user, repo, number
37-
pull_requests.first.committer.name.should == 'Scott Chacon'
37+
expect(pull_requests.first.committer.name).to eq('Scott Chacon')
3838
end
3939

4040
it "should yield to a block" do
4141
yielded = []
4242
result = subject.commits(user, repo, number) { |obj| yielded << obj }
43-
yielded.should == result
43+
expect(yielded).to eq(result)
4444
end
4545
end
4646

0 commit comments

Comments
 (0)