mousu-a

Results 22 comments of mousu-a

@unikounio お待たせいたしました🙇‍♂️ 返信になります! > 併せてヘルパーメソッドを定義するとDRYに書けそうです。 こちらに関しては前述の通りです🙏 > このelse節は0を表示することになると思うので、直接書いた方がDBアクセスがない分効率が良いのかなと思いました。 ご指摘いただいた箇所を日本語に直してみました! ``` users.unretired.advisersがあれば users.unretired.advisers.sizeをリンクにする なければ users.unretired.advisers.sizeを表示する users.unretired.advisersがあれば users.unretired.advisers.sizeをリンクにする なければ 0を表示する ``` 前者はコンテキストが統一されていて「リンクにするかしないか」だけを判断すれば良さそうです。 後者はいきなり0が出てきたので「ん?なんで0?」と多少引っかかる感じがします(ニュアンスで申し訳ないです🙏) 「0を表示する」というのは、実装について多少なりとも把握していないと出てこない発想な気がします。🤔 なのでここで「0」がいきなり出てくると、前述したようにマジックナンバーとなってしまいそうです。👀 (理解するために多少コードを読まなければいけなくなりそう) ですが反面、正直すぐに分かるようなことである気もしていますが...🤔 これらの自分の意見に関してunikounioさんの見解も伺えたらなと思っています🙇‍♂️ 個人的には「最終的な判断はkomagataさんにお伺いする」という形でも良いのかな?と思っているのですがどうでしょうか👀

@unikounio > マジックナンバーに対しては定数やメソッドを用いて対策ができると思います。 なるほどです、こちらに関しては全く頭にありませんでした😅 > 私としては、後学のためにも、ヘルパーメソッドの件も含めて町田さんのご意見をお伺いした上で進められるとよいのかなと思っています。いかがでしょうか? (もしお忙しいようであれば私から町田さんに連絡させていただきますのでおっしゃっていただけますと💡) すみません、ご連絡の方お願い出来ますでしょうか😭🙏 ありがとうございます🙇‍♂️

@machida ご確認ありがとうございますー! @unikounio ご連絡までありがとうございます!🙏 確認していただいてよかったですー! 修正に入りますので今しばらくお待ちください🙇‍♂️

@unikounio お待たせいたしましたー!🙏 ヘルパーメソッドを再導入しDRYにしました。 サイドレビューをお願いいたします🙇‍♂️ > 0がマジックナンバー化してしまう可能性についてのご指摘、なるほどです。 マジックナンバーに対しては定数やメソッドを用いて対策ができると思います。 しかし、これらの対策にもデメリットがあります。 こちらに関してはkomagataさんにお聞きしようと考えていますがどうでしょうか?

@unikounio レビューありがとうございますー! 修正いたしましたので再レビューをお願いいたします🙏

@komagata メンバーの方からApproveいただけましたのでこちらレビューをお願いいたします🙏 一点質問があります🙏 ```ruby # app/views/companies/_user_counts.html.slim - if users.unretired.advisers.present? = link_to_company_users_path(company, target: 'advisers') - else = users.unretired.advisers.size ``` こちらのコードですが、`if users.unretired.advisers.present?`がfalseの時は0なのが確定しているため、else節は0でいいのではないか?(わざわざ`users.unretired.advisers.size`とする必要はなさそう)ということについて議論しています。 komagataさんのご意見を伺いたいです🙏 こちらのやりとりを参照していただけますとわかりやすいと思います。 https://github.com/fjordllc/bootcamp/pull/7479#discussion_r1527431475 https://github.com/fjordllc/bootcamp/pull/7479#issuecomment-2002721906 https://github.com/fjordllc/bootcamp/pull/7479#discussion_r1528017146 https://github.com/fjordllc/bootcamp/pull/7479#issuecomment-2015235946

@komagata すみません、このプルリクにmainブランチの他の人のコミットが混ざってしまっているため今後の対応をお伺いしたいです🙇‍♂️ ## 現状の報告 * 現状どうなっているのか? * mainブランチのコミット内容がプルリクに混ざってしまっている * mainブランチとのコンフリクトが発生している * ログに怪しい記述はないか? * 見てみましたが怪しいログはなさそうでした。。下にログを記述します。 * 推測 * komagataさん, unikounioさんにレビューいただいた時にはこの状態にはなっていなかったと思います。 ### ログ 以下`git reflog`のログになります。 ``` 512a88eb7 (HEAD -> feature/Add-link-to-target-users-for-company-on-companies-page, origin/feature/Add-link-to-target-users-for-company-on-companies-page) HEAD@{0}:...

@komagata お早いお返事ありがとうございます🙇‍♂️ 帰宅後試してみます〜!