Skip to content

melos により自動生成されていた構成ファイルの削除 #49

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

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

warahiko
Copy link
Contributor

概要

  • --dart-define-from-file で env ファイルに定義して環境分け #39 において melos での intellij サポートが無効化されたため、 melos clean/bootstrap を実行する際に差分が発生するようになった
  • 構成ファイルが手動管理されるようになったにも関わらず自動生成ファイルが残っているのに違和感があるため、これを削除する

レビュー観点

  • 削除して問題ないか
  • melos clean/bootstrap で差分が出ないか
  • 他影響がないか

レビューレベル

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

レビュー優先度

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

画像 / 動画

特になし

動作確認手順

melos clean, melos bootstrap の実行

備考

特になし

@warahiko warahiko marked this pull request as ready for review January 10, 2024 09:37
Copy link

Ready for review 🚀

@github-actions github-actions bot requested a review from trm11tkr January 10, 2024 09:37
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.

imo-badge
個人的にはあると地味に便利なので、自動生成の設定はオフにしたものの残していてもいいかなぁと思いましたが、なくても困りはしないので他のお二人に判断お任せします、、!

@warahiko
Copy link
Contributor Author

clean/bootstrap は自分で打ったほうが速いくらいだしアレですが、run catalog はなにかしら別で用意がないといけないとは思いますね 👀
run test については残す場合 report_test にしないとダメそう(自分が rename した際に考慮漏れてました 🙏 )

@Kotaro666-dev
Copy link
Contributor

memo-badge

  • melos clean 実行で AndroidStudio 用の run 設定ファイルが削除される
  • melos clean 実行後に melos bootstrap 実行時に、run 設定ファイルは再生成されない(melos.yamlで生成しない設定をしたため)
  • 最新の main では melos clean で削除される run 設定ファイルの差分が発生してしまっている

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.

@warahiko

PR の作成ありがとうございます!

自分も bootstrap や clean などはコマンドで実行している身なので、両コマンドに関する run 設定ファイルはあってもなくてもいいかなと思います。

ただし、 @warahiko さんが指摘している通り、 run catalog は別途用意したほうが良いですよね。また、テスト実行もコマンド以外で実行したい場合には run test も用意したほうが良さそうですね。
@trm11tkr さんのご意見も聞きたいため、一旦 Approve は しない 状態にしておきます。

@Kotaro666-dev
Copy link
Contributor

Kotaro666-dev commented Jan 11, 2024

memo-badge

run catalog の run 設定ファイルの導入は、以下のチケット進行時に対応するで良さそう。

UI カタログ #5

@warahiko
Copy link
Contributor Author

run test については残す場合 report_test にしないとダメそう

とは書いたものの、これ human readable ではないのでなにかしら別で用意はしたいですね......

Copy link
Contributor

@trm11tkr trm11tkr left a comment

Choose a reason for hiding this comment

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

PR 作成ありがとうございます!

melos での intellij サポートが無効化されたため、 melos clean/bootstrap を実行する際に差分が発生するようになった

自分が対応させていただいたのですが完全に失念しておりました…。
自分もコマンドで実行しているので削除で問題ないです!

trm11tkr added a commit that referenced this pull request Jan 11, 2024
@warahiko
Copy link
Contributor Author

テストに関しては #13 などで、カタログに関しては #5 で対応で良さそうですね!

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

@warahiko warahiko merged commit b4824a8 into main Jan 11, 2024
@warahiko warahiko deleted the feature/remove_unnecessary_configuration_files branch January 11, 2024 09:56
@Kotaro666-dev Kotaro666-dev mentioned this pull request Jan 12, 2024
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.

4 participants