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

[Improve]: *_page.dart でも Provider を生成できるようにする #368

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

morikann
Copy link
Contributor

@morikann morikann commented Jan 9, 2025

概要

レビュー観点

  • build.yaml の記載内容に違和感がないか

レビューレベル

  • Lv1: ぱっとみて違和感がないかチェックして Approve する
  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証して Approve する
  • Lv3: 実際に環境で動作確認したうえで Approve する

レビュー優先度

  • すぐに見てもらいたい ( hotfix など ) 🚀
  • 今日中に見てもらいたい 🚗
  • 今日〜明日中で見てもらいたい 🚶
  • 数日以内で見てもらいたい 🐢

画像 / 動画

見た目に関する変更がないため省略します。

確認したこと

デグレが発生していないか(ナビゲーションが動作しているか)

  • ホームページから Web ページへの遷移
  • デバッグページからナビゲーションデバッグページへの遷移
  • 設定ページからライセンスページへの遷移
  • GitHubList ページから詳細ページへの遷移

自動生成対象ファイルが増えたことによる build_runner 実行時の速度変化

計測方法(簡易的な計測方法です)

  1. melos exec dart run build_runner clean(キャッシュを削除)
  2. melos gen:build --verbose (速度計測)
    • 地味に時間がかかるので before, after で3回ずつのみ計測しています

結果

  • before
    • 1m 13.903s
    • 1m 12.842s
    • 1m 12.882s
  • after
    • 1m 13.629s
    • 1m 14.018s
    • 1m 13.357s

平均

  • before: 1m 13.209s
  • after: 1m 13.668s

差分
0.459秒

備考

build_runner 実行時間の差分が約0.46秒(誤差)と僅かなため、本 PR の変更による影響は許容範囲内と判断しました。

@github-actions github-actions bot added @apps/app Application development @packages/features/debug_mode packages features debug_mode package @packages/samples/github_repository packages samples github_repository package @packages/features/setting packages features setting package labels Jan 9, 2025
Copy link

github-actions bot commented Jan 9, 2025

Visit the preview URL for this PR (updated for commit d190cf4):

https://flutter-mobile-project-template-catalog--pr368-feature-iuguprmf.web.app

(expires Thu, 16 Jan 2025 05:22:53 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 9ea56735a63d07a7cfe62eb204b0528284c37c23

@morikann morikann marked this pull request as ready for review January 9, 2025 07:37
@yumemi-team-review-requester yumemi-team-review-requester bot requested review from a team, tatsutakein and blendthink and removed request for a team January 9, 2025 07:38
Copy link

github-actions bot commented Jan 9, 2025

Ready for review 🚀

Copy link
Member

@blendthink blendthink left a comment

Choose a reason for hiding this comment

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

LGTM です!
ご対応ありがとうございます!

ほんとは↓の該当箇所も合わせて修正しないといけませんが、以前から対応が漏れていたため、別 Issue 作成して対応していただくようにします 🙏
https://github.com/yumemi-inc/flutter-mobile-project-template/blob/main/apps/app/lib/router/README.md

@morikann
Copy link
Contributor Author

@blendthink
レビューありがとうございます!マージします!

↓ でドキュメント更新対応 Issueを起票しました🙏
#370

@morikann morikann merged commit 12ecc17 into main Jan 10, 2025
19 checks passed
@morikann morikann deleted the feature/GH-265/change-build.yaml branch January 10, 2025 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@apps/app Application development @packages/features/debug_mode packages features debug_mode package @packages/features/setting packages features setting package @packages/samples/github_repository packages samples github_repository package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improve]: _page.dartでもProviderを生成できるようにする
2 participants