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

remove: カバレッジコメント削除 #1136

Closed
tarepan opened this issue Mar 22, 2024 · 5 comments · Fixed by #1143
Closed

remove: カバレッジコメント削除 #1136

tarepan opened this issue Mar 22, 2024 · 5 comments · Fixed by #1143
Assignees

Comments

@tarepan
Copy link
Contributor

tarepan commented Mar 22, 2024

追記:提案内容を一部変更(#1136 (comment)

内容

提案概要: カバレッジレポートの廃止

現在の VOICEVOX ENGINE は GitHub Actions を用いたカバレッジ測定・報告をおこなっている。
測定と報告は活用のためにあるが、ここ半年でカバレッジ関連の issue/PR はほぼ存在していない。
現在のカバレッジは 78% であり、これは高いとは言えず、しかしこの値はここ半年以上あまり変わっていない。
PR 上の自動報告コメントも detail タグで畳まれており、積極的な確認もおこなわれていないと推測される。

カバレッジはそれ単体の有用性が高くなく、それを有効活用するリソースも現時点では乏しい。
一方でカバレッジ報告に関する GitHub Actions Workflow 等はそれなりにコード量がある。ゆえに該当コードのメンテコストおよび workflow 全体の理解・管理コストを一定量発生させ続けている。

このような背景から、カバレッジレポートの廃止を提案します。

Pros 良くなる点

  • メンテコスト 0
  • workflow 理解コスト減

Cons 悪くなる点

  • カバレッジ情報が無くなる

実現方法

  • カバレッジ報告機能削除
  • README.md カバレッジバッジ削除
@tarepan tarepan added 機能向上 要議論 実行する前に議論が必要そうなもの labels Mar 22, 2024
@tarepan tarepan changed the title remove: カバレッジ remove: カバレッジ削除 Mar 23, 2024
@Hiroshiba
Copy link
Member

Hiroshiba commented Mar 26, 2024

個人的には削除にどちらかというと賛成です!
単純に自分は見ていないというだけなのですが。あと1回分のコメントになってしまうので通知が飛んできてしまって気になるというのもあります。

特に反対の方が1ヶ月ぐらい現れなかったら消しちゃってもいいのかなと思いました。
cc @y-chan

@tarepan tarepan self-assigned this Mar 26, 2024
@tarepan tarepan added the 状態:実装 実装をおこなっている状態 label Mar 26, 2024
@qryxip
Copy link
Member

qryxip commented Mar 26, 2024

ちょうどCOREの方でカバレッジを導入しようと考えていたところでした (ただしPRでのレポートは無し)。
VOICEVOX/voicevox_core#765

報告だけ削除するという選択肢もあるかなと思いました。ただCOREとENGINEでは色々事情が異なりますし、強い意見ではないです。

@Hiroshiba
Copy link
Member

どうやってカバレッジサービスに登録するのかよく知らないのですが、個人的に気になってるのはカバレッジコメントなので、それ以外は自分も強い意見特にないです!
バッジも提案に含まれてたことに気づきました。これも特に強い意見ないです。

@tarepan
Copy link
Contributor Author

tarepan commented Mar 29, 2024

COREの方でカバレッジを導入 ... PRでのレポートは無し
...
報告だけ削除するという選択肢もある

👍️
CORE 側の情報共有ありがとうございます!
コードの大半は PR コメント報告(カバレッジコメント)用であるため、カバレッジコメントだけ削除する方向性は大いにありだと感じました。

気になってるのはカバレッジコメント

👍️
PR 通知の観点でカバレッジコメントは削除寄り、ということですね。


  • カバレッジコメント削除
    • workflow 理解コストの大半が消滅
    • 過剰通知が消滅
  • カバレッジサービス & バッジ維持
    • カバレッジ情報へはアクセス可能

であるため、カバレッジコメントに限った削除が妥当だと感じます。
提案内容をこちらへ変更します。

@Hiroshiba
カバレッジコメント削除であれば #1143 merge 後に問題発生しても即座に切り戻し可能です。
そして実際に merge してカバレッジコメントを消せば「やっぱりカバレッジ報告が PR コメントとして欲しい」という意見がすぐに出てくると考えられます。意見が出ない場合、実利用者がいなかった、ということです。
よって 1 ヶ月の猶予を取らず #1143 review & merge が良いと考えます。
この方向性でどうでしょうか?

@Hiroshiba
Copy link
Member

まあ確かに速い方が良い気もしました。

一番手っ取り早いのは @y-chan さんの意見を聞くことだと思います!
@y-chan カバレッジコメントの廃止についてコメントいただけると・・・!

@tarepan tarepan changed the title remove: カバレッジ削除 remove: カバレッジコメント削除 Apr 9, 2024
@tarepan tarepan removed 要議論 実行する前に議論が必要そうなもの 状態:実装 実装をおこなっている状態 labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants