Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Cure.peace.pikarin_janken (singleton method) #39

Merged
merged 5 commits into from
Aug 28, 2014
Merged

Add Cure.peace.pikarin_janken (singleton method) #39

merged 5 commits into from
Aug 28, 2014

Conversation

zonuexe
Copy link
Contributor

@zonuexe zonuexe commented Aug 24, 2014 

% ruby -rubicure -e'Cure.peace.pikarin_janken'
ピカピカピカリン
ジャンケンポン！
（チョキ）
% ruby -rubicure -e'Cure.peace.janken'
ピカピカピカリン
ジャンケンポン！
（グー）

houndci-bot
houndci-bot reviewed Aug 24, 2014
View reviewed changes
lib/rubicure.rb
@@ -28,6 +28,18 @@ def self.method_missing(name, *args)
super
end
end

class << self.peace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cure.peace と Cure.cure_peace で機能差分出るのは気持ち悪いので両方対応していただきたく。（別インスタンスなので。。。）

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど…

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.18%) when pulling 4c57f48 on zonuexe:feature/pikarin_janken into c65a205 on sue445:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.15%) when pulling b758e42 on zonuexe:feature/pikarin_janken into c65a205 on sue445:master.

sue445
sue445 reviewed Aug 24, 2014
View reviewed changes
lib/rubicure.rb
（%s）
EOF
def pikarin_janken
print_by_line MESSAGE % HANDS.sample
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

引数ある場合はメソッド呼び出しの括弧ほしいです

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.14%) when pulling bbd2860 on zonuexe:feature/pikarin_janken into c65a205 on sue445:master.

houndci-bot
houndci-bot reviewed Aug 24, 2014
View reviewed changes
lib/rubicure.rb
[Cure.peace, Cure.cure_peace].each do |peace|
class << peace
HANDS =
(['グー'] * 13) +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.14%) when pulling e402aee on zonuexe:feature/pikarin_janken into c65a205 on sue445:master.

@sue445
Copy link
Owner

sue445 commented Aug 25, 2014

t_wada_sabanna

メソッドがエラーにならないかどうかのテストだけでもほしいですｗ

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.14%) when pulling 4795b51 on zonuexe:feature/pikarin_janken into a27b60c on sue445:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) when pulling fe5eb71 on zonuexe:feature/pikarin_janken into a27b60c on sue445:master.

sue445
sue445 reviewed Aug 28, 2014
View reviewed changes
spec/girl_spec.rb
@@ -157,4 +157,13 @@
it { expect{ subject }.to raise_error NoMethodError }
end
end

describe "cure_peace#pikarin_janken" do
let(:girl){ Cure::peace }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

メソッド呼び出しでコロンは違和感あるのでドットで

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

拝承

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) when pulling aed2b52 on zonuexe:feature/pikarin_janken into a27b60c on sue445:master.

sue445 added a commit that referenced this pull request Aug 28, 2014
@sue445
Merge pull request #39 from zonuexe/feature/pikarin_janken
d46ca44 
Add Cure.peace.pikarin_janken (singleton method)
@sue445 sue445 merged commit d46ca44 into sue445:master Aug 28, 2014
@sue445
Copy link
Owner

sue445 commented Aug 28, 2014

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zonuexe @coveralls @sue445 @houndci-bot