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

未使用ファイルを削除 #8082

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

Ryooo-k
Copy link
Contributor

@Ryooo-k Ryooo-k commented Sep 20, 2024

Issue

概要

使用していないパーシャルファイルがあるため、以下の変更・確認を行いました。

  • app/views/practices/_tab.html.slimを削除
  • app/views/practices/_tab.html.slimがどこからも使用されていないかの確認

変更確認方法

  1. chore/delete-_tab.html.slimをローカルに取り込む
  2. ルートディレクトリに移動する
  3. ls app/views/practicesを実行し_tab.html.slim が存在していないかを確認する

Screenshot

変更前

スクリーンショット 2024-09-19 18 52 48

変更後

スクリーンショット 2024-09-19 18 51 08

未使用ファイルの確認

方法

ターミナルのgrepコマンドで文字列検索を行い、render 'tab'などのような記載がないかを確認しました。

スクリーンショット 2024-09-20 20 48 44
  1. viewsディレクトリ以下にある全てファイルを対象にpractices/tab を検索
  2. viewsディレクトリ以下にある全てファイルを対象にpractices/_tab を検索
  3. app/views/practicesディレクトリ内の.html.slimファイルを対象に tab を検索

結果

1と2の条件の場合はヒット無し、3の条件の場合render page_tabsがヒットしましたが、今回の削除したファイルではありません。
上記より、_tab.html.slimは未使用ファイルだと判断します。

@Ryooo-k Ryooo-k self-assigned this Sep 20, 2024
@Ryooo-k Ryooo-k marked this pull request as ready for review September 20, 2024 12:10
@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Sep 20, 2024

@ayu-0505
初レビュー依頼です!ご確認いただけますでしょうか🙏

@ayu-0505
Copy link
Contributor

@Ryooo-kさん、お疲れ様です〜🍵レビュー承知しました!
連休中はFBCに使える時間が少なく、場合によっては3〜4日ほどかかるかもしれません。
なるべく早めにお返しするようにしますが、もしお急ぎの場合は遠慮なくおっしゃってください🙏

@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Sep 22, 2024

@ayu-0505

すみません💦テストが通ったのを確認してなかったので、テスト通過後改めてご連絡しますので、レビューはもう少し待っていただきたいです🙇申し訳ありません💦

@ayu-0505
Copy link
Contributor

@Ryooo-kさん
了解です!レビューOKになりましたらまたお知らせください〜。

@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Sep 26, 2024

@ayu-0505
ayuさんのおかげでテストが問題ないこと確認できましたー!ありがとうございます✨
レビューをお願いいたします🙇

@ayu-0505
Copy link
Contributor

@Ryooo-kさん、よかったです〜✨
明日中にレビューしたいと思いますので、すみませんが少々お待ちください🙏

@ayu-0505
Copy link
Contributor

ayu-0505 commented Sep 27, 2024

@Ryooo-kさん、お疲れ様です🍵お待たせいたしました〜。

未使用ファイルの確認について

views内のチェックでほぼ大丈夫かなとは思うのですが、使用しようと思えばできることから
app/controllers内、app/mailers内も念の為チェックしたほうが良いかも、と考えます👀
Action Mailer の基礎 ※こちらにパーシャルも利用可能と書かれていました。

変更確認方法の記載について

こちらは私もつい先日教えていただいた書き方なのですが、以下のように書くとレビュワーに親切かもしれません。
ご参考までにどうぞ🙏(pull/PRid/head:ブランチ名で該当PRのブランチを指定できる)

  1. chore/delete-_tab.html.slimをローカルに取り込む
    1. git fetch origin pull/8082/head:chore/delete-_tab.html.slim
    2. git switch chore/delete-_tab.html.slim

@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Sep 28, 2024

@ayu-0505
早速ご確認ありがとうございます!

app/controllers内、app/mailers内も念の為チェックしたほうが良いかも、と考えます👀

確かにおっしゃる通りcontrollersとmailers内もチェックした方が良さそうですね!教えていただきありがとうございます。
再度チェック後、ご連絡いたします。

変更確認方法の記載について

こちらも教えていただきありがとうございます!
確かに打つコマンドそのものを記載していた方が親切ですね。ありがとうございます👍

@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Oct 1, 2024

@ayu-0505

未使用ファイルの確認に関して、appディレクトリ内の全てのファイルに対してpractices/tab およびpractices/_tabの文字列検索を行いました。

スクリーンショット 2024-10-01 10 49 00

結果としては文字列がヒットしなかったため、問題ないと判断します!
お手数ですが、ご確認お願いいたします🙇

@ayu-0505
Copy link
Contributor

ayu-0505 commented Oct 1, 2024

@Ryooo-kさん、お疲れ様です🍵
確認いたしました〜。問題ないと思われますのでApproveにさせていただきます!

以下の内容については、ご存知でしたらスルーしてください🙏

  • komagataさんにレビュー依頼する際にも、受講生に依頼したようにreviewersにkomagataさんを追加するそうです。
    Good First Issue 攻略・その2には記載がなく、私は最初うっかり忘れてしまっていたので一応の共有です。)

@Ryooo-k Ryooo-k requested a review from komagata October 2, 2024 00:23
@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Oct 2, 2024

@ayu-0505
ご確認ありがとうございます🙇

komagataさんにレビュー依頼する際にも、受講生に依頼したようにreviewersにkomagataさんを追加するそうです。

こちらも教えていただきありがとうございます!

@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Oct 2, 2024

@komagata
お疲れ様です!
メンバーレビューでApproveいただきましたので、ご確認お願いいたします。

@komagata
Copy link
Member

komagata commented Oct 2, 2024

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

@komagata komagata merged commit b61a914 into main Oct 2, 2024
4 checks passed
@komagata komagata deleted the chore/delete-_tab.html.slim branch October 2, 2024 12:08
@github-actions github-actions bot mentioned this pull request Oct 2, 2024
15 tasks
@Ryooo-k
Copy link
Contributor Author

Ryooo-k commented Oct 2, 2024

ご確認ありがとうございます!

@github-actions github-actions bot mentioned this pull request Oct 3, 2024
17 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