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.

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

mentor-practices.vueを非vue化 #7484

Merged
merged 5 commits into from
Mar 9, 2024

Conversation

ghost
Copy link

@ghost ghost commented Mar 3, 2024

Issue

概要

mentor-practices.vueを削除し、slimでの実装に変更しました。

変更確認方法

  1. feature/convert-mentor-practices-vue-to-slimをローカルに取り込む
  2. ローカル環境を起動し、管理者ユーザーでログインする
  3. /mentor/practices に移動し、プラクティスの一覧が正常に表示されていることを、mainブランチ等と比較して確認する
  4. プラクティス一覧の中のリンクが正しく機能していることを確認する

Screenshot

変更前

image

変更後

image

@ghost ghost self-assigned this Mar 3, 2024
@ghost ghost requested a review from reckyy March 5, 2024 03:29
@ghost
Copy link
Author

ghost commented Mar 5, 2024

@reckyy
お疲れ様です。
こちらのPRのレビューをお願いしてもよろしいでしょうか🙏
お手すきの際で構いませんので、よろしくお願いいたします🙇‍♂️

@reckyy
Copy link
Contributor

reckyy commented Mar 6, 2024

@taco-nantai
お疲れ様です!
ちょっとバタバタしてて、レビューをお戻しするのが遅くなってしまいそうなので、他の方に依頼をお願いしてもよろしいでしょうか。 🙇
せっかくレビュー依頼いただいたのに、申し訳ございません。
よろしくお願い致します。

@ghost ghost removed the request for review from reckyy March 6, 2024 05:15
@ghost
Copy link
Author

ghost commented Mar 6, 2024

@reckyy
かしこまりました!
お手数おかけして申し訳ありません🙇‍♂️

@ghost ghost requested a review from yocchan-git March 6, 2024 05:19
@ghost
Copy link
Author

ghost commented Mar 6, 2024

@yocchan-git
お疲れ様です。
こちらのPRのレビューをお願いしてもよろしいでしょうか🙏
お手すきの際で構いませんので、よろしくお願いいたします🙇‍♂️

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

動作としては良さそうでした!

ただ、1点だけ非同期で処理しているのが疑問に思いました🤔
別に非同期にするようなページでもなさそうですし、Hotwireに移行するにしてもSlimの方がやりやすいと思います。

issueを見る限り良さそうで、mainと同じ動作なのですが、念の為今日のMTGで聞いていただいてもよろしいでしょうか?
よろしくお願いいたします🙇‍♂️

@yocchan-git
Copy link
Contributor

また、下書きからOpenにしていただけますと🙏
僕の画面だけかもですが...

@ghost ghost marked this pull request as ready for review March 6, 2024 05:49
@ghost ghost force-pushed the feature/convert-mentor-practices-vue-to-slim branch from 16a4daa to aeca0c0 Compare March 6, 2024 14:02
@ghost ghost requested a review from yocchan-git March 6, 2024 15:06
@ghost
Copy link
Author

ghost commented Mar 6, 2024

@yocchan-git
レビューありがとうございます。
指摘を頂いた個所を修正したので、再度確認をお願いします🙇‍♂️

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

1点コメントしましたが、LGTMです🎉

対応お疲れ様でした!
コメントした点を修正したら、komagataさんへお願いします〜🙏

app/controllers/mentor/practices_controller.rb Outdated Show resolved Hide resolved
Co-authored-by: よっちゃん <143366399+yocchan-git@users.noreply.github.com>
@ghost
Copy link
Author

ghost commented Mar 7, 2024

@yocchan-git
指摘を頂いた箇所をコミットしました👍
レビューして頂きありがとうございました🙇‍♂️

@ghost ghost requested a review from komagata March 7, 2024 01:51
@ghost
Copy link
Author

ghost commented Mar 7, 2024

@komagata
お疲れ様です。
PRにapproveをいただいたので、レビューをお願いします🙇‍♂️

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認させて頂きました。OKです〜🙆‍♂️

@komagata komagata merged commit c48fe26 into main Mar 9, 2024
4 checks passed
@komagata komagata deleted the feature/convert-mentor-practices-vue-to-slim branch March 9, 2024 17:13
@github-actions github-actions bot mentioned this pull request Mar 9, 2024
29 tasks
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.

3 participants