Closed kerochelo closed 1 year ago
@nakata59 MVPリリースお疲れ様でした:tada:
下記の観点でレビューしております。
APIを投げて独自のロジックを使って情報の精査している部分はいいですね。 ただitems_controllerがfat controllerになっているので、APIでのsearch周りの処理をserviceクラスに切り出すなど、下記の記事を参考にこの部分をリファクタしてあげるといいかなと思います! https://techracho.bpsinc.jp/hachi8833/2021_10_05/112108
またがっつりRailsのメリットを用いている機能があまりないので、その辺りを使って機能を修正、追加機能の実装をするといいかなと思います。 このアプリをベースに考えると下記の機能が一目置かれる技術の実装としていいかなと思います!
rubocopで静的コードチェックをしておいてください。 コードの見た目が整っていない箇所が多々あるので、レビュー時にみやすいようなコードにしておきましょう!
下記の部分が気になったので本リリースまでに修正しておきましょう。
READMEに記載している課題解決の機能は果たせているので、上記のフィードバック内容を進めていただき、エンジニアとしての技術の見せ方やサービスとしての見せ方を追加・修正できれば、就活フェーズに入って大丈夫です。
こちらの内容を確認いただき、それぞれissueでタスクを作成できたらcloseしてもらって構いません。 以上よろしくお願いします。
@kerochelo お疲れ様です。 リアクション遅くなり誠に申し訳ございません。 忙しい中、レビューいただきありがとうございました いただいたアドバイスから下記issueを作成しました。
issueを作成しましたのでこちらのissueはcloseさせていただきます。
@nakata59 MVPリリースお疲れ様でした:tada:
下記の観点でレビューしております。
就活において十分な技術力を示せる機能を実装しているか
APIを投げて独自のロジックを使って情報の精査している部分はいいですね。 ただitems_controllerがfat controllerになっているので、APIでのsearch周りの処理をserviceクラスに切り出すなど、下記の記事を参考にこの部分をリファクタしてあげるといいかなと思います! https://techracho.bpsinc.jp/hachi8833/2021_10_05/112108
またがっつりRailsのメリットを用いている機能があまりないので、その辺りを使って機能を修正、追加機能の実装をするといいかなと思います。 このアプリをベースに考えると下記の機能が一目置かれる技術の実装としていいかなと思います!
rubocopで静的コードチェックをしておいてください。 コードの見た目が整っていない箇所が多々あるので、レビュー時にみやすいようなコードにしておきましょう!
UIでユーザーが迷わないような導線を作れているか・ユーザーが使いたくなるようなサービス設計ができているか
下記の部分が気になったので本リリースまでに修正しておきましょう。
READMEに記載している課題解決の機能は果たせているので、上記のフィードバック内容を進めていただき、エンジニアとしての技術の見せ方やサービスとしての見せ方を追加・修正できれば、就活フェーズに入って大丈夫です。
こちらの内容を確認いただき、それぞれissueでタスクを作成できたらcloseしてもらって構いません。 以上よろしくお願いします。