Skip to content

Conversation

@uni-kakurenbo
Copy link
Contributor

なぜこの PR を入れたいのか

closes: #4427

PR を出す前の確認事項

  • (機能の追加なら)追加することの合意がチームで取れている
    • 取れていない場合はチェックを外して PR にすれば OK
  • 動作確認ができている
  • 自分で一度コードを眺めて自分的に問題はなさそう

@uni-kakurenbo uni-kakurenbo requested a review from a team September 25, 2025 10:10
@uni-kakurenbo uni-kakurenbo self-assigned this Sep 25, 2025
@uni-kakurenbo uni-kakurenbo added the bug Something isn't working label Sep 25, 2025
@github-actions
Copy link

@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.53%. Comparing base (b127b2a) to head (0cbcea1).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...iewContent/composables/useChannelMessageFetcher.ts 0.00% 8 Missing ⚠️
...ew/ClipsViewContent/composables/useClipsFetcher.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4828   +/-   ##
=======================================
  Coverage   13.53%   13.53%           
=======================================
  Files         704      704           
  Lines       31729    31729           
  Branches      677      677           
=======================================
  Hits         4294     4294           
  Misses      27423    27423           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@uni-kakurenbo uni-kakurenbo requested a review from cp-20 October 23, 2025 05:23
Copy link
Contributor

Choose a reason for hiding this comment

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

asc を名前から外す理由は?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

desc には何もついていないのに asc だけ明示しているのに若干違和感があった (おそらく元々の実装で reverse を非破壊と勘違いしていて、代入し直すための変数として message と衝突しない messageAsc が選ばれた?) のと、すぐ近くで order: 'asc' を明示しているのでわざわざ明示する意味が薄いと感じたからです

asc が付いている方が良さそうなら戻します

Copy link
Contributor

Choose a reason for hiding this comment

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

desc と asc は区別した方が後々分かりやすいような気がするな (useChannelMessageFetcher はそれなりに複雑な実装だと思うし) 両方付ける用に統一して toReversed() で非破壊に操作すると良いかも

Copy link
Contributor Author

Choose a reason for hiding this comment

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

たしかにです、そうします

@uni-kakurenbo uni-kakurenbo requested a review from cp-20 October 23, 2025 07:55
Copy link
Contributor

@cp-20 cp-20 left a comment

Choose a reason for hiding this comment

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

good

@uni-kakurenbo uni-kakurenbo force-pushed the fix/clipped-messages-ordering branch from 4b54c82 to 78641a6 Compare October 23, 2025 08:05
@uni-kakurenbo uni-kakurenbo force-pushed the fix/clipped-messages-ordering branch from 78641a6 to 0cbcea1 Compare October 23, 2025 08:06
@uni-kakurenbo uni-kakurenbo merged commit 9231d7c into master Oct 23, 2025
11 checks passed
@uni-kakurenbo uni-kakurenbo deleted the fix/clipped-messages-ordering branch October 23, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

クリップ内の時系列が逆になっている

3 participants