Skip to content

Sites#7

Open
iwhurtafly wants to merge 4 commits intomasterfrom
sites
Open

Sites#7
iwhurtafly wants to merge 4 commits intomasterfrom
sites

Conversation

@iwhurtafly
Copy link
Copy Markdown
Member

SiteモデルにValidationを追加しました。その他、コードに微調整が入っているのは、今回は了承下さい。また、前回strong parametersを検討していた際に追加した「validates_url_format_of.rb」を削除し忘れていました。次回のコミットで削除します。今回のコミット以降は、奇麗な単位でコミットして行けると思います。(次回のコミットには、「validates_url_format_of.rb」を削除したという履歴が残ってしまうでしょうが。。)

最後に2つ質問させて下さい。

・今回、url_is_herokuというメソッドを定義していますが、テストの書き方が分かりませんでした。
 アドバイス頂けますか?
・今回のプルリクエストが取り込まれた場合、今のブランチは削除して、別ブランチで作業ですよね?
 (前にも聞いたかもしれませんが、失念してしまいました。。)

@ppworks
Copy link
Copy Markdown
Contributor

ppworks commented Sep 8, 2013

・今回、url_is_herokuというメソッドを定義していますが、テストの書き方が分かりませんでした。

url_is_heroku?ですかね。

Resolv.getaddress が肝になっているので、この部分を stub してはどうでしょう?

@ppworks
Copy link
Copy Markdown
Contributor

ppworks commented Sep 8, 2013

・今回のプルリクエストが取り込まれた場合、今のブランチは削除して、別ブランチで作業ですよね?

ですです!実現したい価値や、機能にフューチャーした内容でブランチを切りましょう。

@iwhurtafly
Copy link
Copy Markdown
Member Author

・今回、url_is_herokuというメソッドを定義していますが、テストの書き方が分かりませんでした。

url_is_heroku?ですかね。

Resolv.getaddress が肝になっているので、この部分を stub してはどうでしょう?

url_is_heroku?です。stubの件、なるほどですね。今回はこの部分のテストは画面からのテストにしようかと思います。今後、テストを書く時の参考にさせて頂きます。

ですです!実現したい価値や、機能にフューチャーした内容でブランチを切りましょう。

OKです!

先に進みたいので、このプルリクエストをマージしますか?

@ppworks
Copy link
Copy Markdown
Contributor

ppworks commented Sep 9, 2013

今回はこの部分のテストは画面からのテストにしようかと思います。

テストで何を担保したいかを意識してればよいですが、ここはモデルのテスト欲しいですね。

先に進みたいので、このプルリクエストをマージしますか?

ok!

@ppworks
Copy link
Copy Markdown
Contributor

ppworks commented Sep 10, 2013

url_is_heroku?
が色々いけてないのでリファクタリングしたいですね。こんど一緒にリファクタリングしますかー

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants