xpjp / xpfiat-bot

XP JPのディスコード上で動作しているボットです。 XPをJPYに変換したりする機能があります。
MIT License
20 stars 5 forks source link

コマンド毎にファイル分割 #79

Closed harigel closed 6 years ago

harigel commented 6 years ago

何を解決するのか

66 メインファイルが肥大化してきたので、コマンド毎等でファイルを分割した。

レビューポイント

diffがひどいことになってしまっております…すみません docomo系、IBM系のAPI keyが無いものについてはテストできてません…すみません 時間切れで関数がxp_fiat.rbに残ってしまっていますが、別のプルリクで処理する予定です。 (手伝ってくれてもいいのよ

@xpjp/ruby-reviewer 恐縮ですがよろしくお願いします!

Diaboro87 commented 6 years ago

ファイル数多いので、明日現物見ながらやりますが、CI通ってないので確認して下さい。

harigel commented 6 years ago

確認します!

harigel commented 6 years ago

rubocop黙らせました! 他の方の書いた部分も僭越ながら修正しちゃいました。

toshikidoi commented 6 years ago

大作ありがとうございます! 出来るだけ早くレビューします〜〜〜💪

harigel commented 6 years ago

すみません改めてテストしてたらデグレを見つけてしまいました。 後出しですみませんがa47b3f6このコミットで修正しました。 その他、ついでにコメントの消し忘れやrequire周りの整理等行いました。 よろしくお願いします。

mc-chinju commented 6 years ago

実装お疲れ様です!

PR が大きいので、debug gem に関する部分を別で出しました。 https://github.com/xpjp/xpfiat-bot/pull/81

こちらはファイル分割の PR に絞りましょう 👍

harigel commented 6 years ago

@mc-chinju ありがとうございます!了解です。 えと、こちらはgem部分revertしたほうがいいのでしょうか?

mc-chinju commented 6 years ago

@harigel そですね!お願いしますー。

harigel commented 6 years ago

pry戻しました。

harigel commented 6 years ago

Squash and mergeという機能があるとお聞きしました。 本質と関係ないコミットが多くあるので、mergeいただく際はまとめてSquashしていただきたいですー

toshikidoi commented 6 years ago

何点かコメントしましたが、提案レベルの物なので、目を通して頂ければOKです〜。 ということで、LGTMです🎉

まだレビューされてない方もいらっしゃるかなということで、いったんReady to mergeは貼らないでおきますね〜。

mc-chinju commented 6 years ago

3人通してるので Ready to merger フラグに変えておきました。

harigel commented 6 years ago

@xpjp/ruby-reviewer みなさんありがとうございました!

@p-suke マージでたぶん他プルリクと盛大にコンフリクトすると思うので、その際はお声がけください。これを最後に回してもらうのがいいかも…?

chipstar commented 6 years ago

すみません。 コンフリクトしてしまいました。。。

Asuforce commented 6 years ago

他のプルリクとコンフリクトするかもしれませんが、新機能は今の master ブランチから作成されてしまうので、永遠にマージできなくなってしまいます。なので、このプルリクを先にマージすべきだと思います。

harigel commented 6 years ago

すみません今日はあまり作業できなそうです。 とりあえず夕方で良ければコンフリクト解消までします。

chipstar commented 6 years ago

@Asuforce すみません次回はこのプルリクを先にマージしますね。

chipstar commented 6 years ago

@harigel ありがとうございます。手の空いているときにお願いします。

chipstar commented 6 years ago

@harigel 試しにコンフリクト解消してみました。間違ってないですかね??

Asuforce commented 6 years ago

@p-suke よさそうです!

chipstar commented 6 years ago

@Asuforce ありがとうございます!masterにマージしました。