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

開催前の定期イベントは次回開催日を表示しない #7995

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

MikotoMakizuru
Copy link
Contributor

@MikotoMakizuru MikotoMakizuru commented Aug 5, 2024

Issue

概要

  • 開催前の定期イベントは次回開催日を表示しないようにしました。

変更確認方法

  1. feature/next-date-not-displayed-for-wip-regular-event をローカルに取り込む
  2. ログインして、WIP の定期イベントを作成
  3. 2 で作成した WIP の定期イベントの詳細ページにアクセス
  4. 次回開催日が「イベント編集中のため次回開催日は未定です」となっていることを確認する

Screenshot

変更前

スクリーンショット 2024-08-07 21 46 02

変更後

スクリーンショット 2024-08-07 21 46 40

@MikotoMakizuru MikotoMakizuru self-assigned this Aug 5, 2024
@MikotoMakizuru MikotoMakizuru force-pushed the feature/next-date-not-displayed-for-wip-regular-event branch 2 times, most recently from 2f1d292 to 8947f20 Compare August 8, 2024 12:55
@MikotoMakizuru
Copy link
Contributor Author

@masyuko0222
こんにちは、こちらのコードレビューを依頼することは可能でしょうか。

@masyuko0222
Copy link
Contributor

@MikotoMakizuru
承知しましたー。来週末くらいになってしまうかもしれないので、急ぎでしたら他の方にお声がけ頂ければです🙇‍♂️

@MikotoMakizuru
Copy link
Contributor Author

@masyuko0222
ありがとうございます!日程の件問題ありません。
よろしくお願いいたします。🙇‍♂️

@masyuko0222 masyuko0222 self-requested a review August 8, 2024 14:14
Copy link
Contributor

@masyuko0222 masyuko0222 left a comment

Choose a reason for hiding this comment

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

動作確認・コード共に問題ないと思います~!LGTM!

コードではないのですが、コミットの「テストコード修正」というコメントと、同じものが何個もあるのはよろしくないかな~と思います。
「コードレビューの修正(1回目)」みたいなコミットメッセージを避ける

全て同じ理由でテストコードを修正しているように見えるので、1つのコミットにまとめつつ、意味のあるコメントを書きたいかなと思いました~。

@MikotoMakizuru
Copy link
Contributor Author

@masyuko0222
レビューありがとうございます!🙌

全て同じ理由でテストコードを修正しているように見えるので、1つのコミットにまとめつつ、意味のあるコメントを書きたいかなと思いました~。

承知しました。
コミット一つにまとめつつ、周りが見てもわかる具体的コミットメッセージを心がけたいと思います。📝

@MikotoMakizuru
Copy link
Contributor Author

@komagata
レビューよろしくお願いいたします

@@ -155,3 +155,14 @@ regular_event32:
user: komagata
category: 0
published_at: "2023-08-01 00:00:00"

regular_event33:
Copy link
Member

Choose a reason for hiding this comment

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

fixturesに書くと全てのテストケースでDBにINSERT/DELETEされるので、一つのテストケースでしか使わないようなデータはそのテストの中で作るようにしてみてください〜

Copy link
Contributor Author

Choose a reason for hiding this comment

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

テストデータを個別のテストの中で作成するようにしました。

@MikotoMakizuru MikotoMakizuru force-pushed the feature/next-date-not-displayed-for-wip-regular-event branch from 3032cdf to d74acc1 Compare August 18, 2024 06:30
@komagata
Copy link
Member

@MikotoMakizuru

image

同じコミットログが続いているようなので、後から見た人が分かりやすいようにまとめたり、分割したり、整理してみてください〜。

@MikotoMakizuru MikotoMakizuru force-pushed the feature/next-date-not-displayed-for-wip-regular-event branch from d74acc1 to 4347ec7 Compare August 18, 2024 16:24
@MikotoMakizuru MikotoMakizuru force-pushed the feature/next-date-not-displayed-for-wip-regular-event branch from 4347ec7 to 686e4cf Compare August 18, 2024 16:31
@MikotoMakizuru MikotoMakizuru force-pushed the feature/next-date-not-displayed-for-wip-regular-event branch from 16a1b67 to 409326e Compare August 19, 2024 14:15
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 33d3bac into main Aug 21, 2024
4 checks passed
@komagata komagata deleted the feature/next-date-not-displayed-for-wip-regular-event branch August 21, 2024 00:27
@github-actions github-actions bot mentioned this pull request Aug 21, 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