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

PR の解析・テストを行うワークフロー追加 #37

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

warahiko
Copy link
Contributor

概要

close #7

レビュー観点

  • PR 作成時に解析・テストを行うワークフローとなっているか
  • 解析・テストの範囲に過不足ないか

レビューレベル

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

レビュー優先度

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

@warahiko warahiko changed the title Feature/add check action PR の解析・テストを行うワークフロー追加 Dec 26, 2023
Copy link
Contributor Author

@warahiko warahiko left a comment

Choose a reason for hiding this comment

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


- name: Setup melos
run: |
ln -s $FLUTTER_ROOT .fvm/flutter_sdk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

melos.yaml で指定しているため必要

@@ -5,22 +5,20 @@ targets:
# https://github.com/dart-lang/build/blob/master/docs/faq.md#how-do-i-avoid-running-builders-on-unnecessary-inputs
generate_for:
include:
- lib/model/*.dart
- lib/**/model/*.dart
- lib/{model,**/model}/**.dart
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lib/model などが必要かどうかは微妙?
マルチパッケージにするなら直下に生えることはあるかも、とは思った

Copy link
Member

Choose a reason for hiding this comment

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

ここどうするかは悩ましいですが、いったんこのままでいいかなと思いました、、!
(というか、このような中括弧をつかった書き方ができるのですね)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

中で https://pub.dev/packages/glob を使ってるみたいで、このページ参考にしました!

@warahiko warahiko marked this pull request as ready for review December 27, 2023 05:10
Copy link

Ready for review 🚀

melos.yaml Outdated Show resolved Hide resolved
fatal-infos: true

- name: Run test
run: melos run test --no-select
Copy link
Member

Choose a reason for hiding this comment

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

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.

細かいところをコメントしているものの、特に問題はないと思うので Approve させていただきます!
ご対応ありがとうございます 🎉

LGTM

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!


scripts:
report_test:
exec: flutter test --no-pub --machine > test_report.log
Copy link
Contributor

Choose a reason for hiding this comment

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

memo-badge

--machine オプションで json 形式で標準出力する。

気になる点

flutter test --help で表示されるオプションに --machine オプションが記載されていない。
その一方で、 --reporter というオプションでテスト結果の出力形式を json に指定できる模様。
ただし、--machine--reporter オプションで出力される内容に差異がありそう。

-r, --reporter                                               Set how to print test results. If unset, value will default to either compact or expanded.

          [compact]                                          A single line that updates dynamically (The default reporter).
          [expanded]                                         A separate line for each update. May be preferred when logging to a file or in continuous integration.
          [github]                                           A custom reporter for GitHub Actions (the default reporter when running on GitHub Actions).
          [json]                                             A machine-readable format. See: https://dart.dev/go/test-docs/json_reporter.md

@Kotaro666-dev Kotaro666-dev self-requested a review January 4, 2024 05:31
Copy link
Contributor

@Kotaro666-dev Kotaro666-dev left a comment

Choose a reason for hiding this comment

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

対応ありがとうございました!
問題ないと思います!

LGTMeow

Copy link
Contributor

@morikann morikann left a comment

Choose a reason for hiding this comment

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

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

@warahiko
Copy link
Contributor Author

@blendthink @Kotaro666-dev @morikann
コンフリクトがあったので解消しました、確認お願いしたいです! 🙏

@Kotaro666-dev Kotaro666-dev self-requested a review January 10, 2024 03:23
Copy link
Contributor

@Kotaro666-dev Kotaro666-dev left a comment

Choose a reason for hiding this comment

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

コンフリクト解消ありがとうごいました!
LGTM!!

@warahiko
Copy link
Contributor Author

レビューありがとうございました、こちらマージします!

@warahiko warahiko merged commit bbbb930 into main Jan 10, 2024
2 checks passed
@warahiko warahiko deleted the feature/add_check_action branch January 10, 2024 03:57
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.

GitHub Actions で静的解析・テスト
4 participants