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

整理: GitHub workflow step の命名規則を統一 #1139

Merged
merged 14 commits into from
May 1, 2024

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Mar 23, 2024

内容

概要: GitHub workflow step の命名規則を統一してリファクタリング

GitHub workflow の step 名(jobs.<job_id>.steps[*].name)はそのステップの概要を示し、GitHub の workflow UI 上に表示される。理想的にはこの UI 上で「どの段階がコケたのか(例: 環境構築、ビルド、テスト)」を知りたい。
しかし現在の VOICEVOX ENGINE は step 名に命名規則がないため、一見してわからない。
もし命名規則があれば「環境構築がそもそも上手く行っていない」「新機能を入れたらテストが fail した」と直感的に理解できる。

このような背景から、GitHub workflow step の命名規則統一によるリファクタリングを提案します。

具体的には step を以下の大枠に分類し、step 名の最初の単語でこれを明示しました:

  • 環境構築: Set up ... (例: Set up Python dependencies)
  • ビルド: Build ... (例: Build the application as 7z archives)
  • テスト: Test ... (例: Test format)
  • デプロイ: Deploy ... (例: Deploy documents to GitHub Pages)

一部の steps は上記段階の間にあるなどの事情で名称を維持しています。

関連 Issue

無し

Notes

この PR の Actions Jobs を見ると恩恵がわかると思います(例: test (ubuntu-20.04, 3.11.3)

@tarepan tarepan requested a review from a team as a code owner March 23, 2024 07:29
@tarepan tarepan requested review from Hiroshiba and removed request for a team March 23, 2024 07:29
Copy link

github-actions bot commented Mar 23, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 277 122 coverage-56%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/app/init.py 0 0 coverage-100%
voicevox_engine/app/dependencies.py 9 0 coverage-100%
voicevox_engine/app/routers/init.py 0 0 coverage-100%
voicevox_engine/app/routers/preset.py 37 4 coverage-89%
voicevox_engine/app/routers/setting.py 23 3 coverage-87%
voicevox_engine/app/routers/speaker.py 59 5 coverage-92%
voicevox_engine/app/routers/tts_pipeline.py 121 29 coverage-76%
voicevox_engine/app/routers/user_dict.py 60 29 coverage-52%
voicevox_engine/cancellable_engine.py 97 75 coverage-23%
voicevox_engine/core/init.py 0 0 coverage-100%
voicevox_engine/core/core_adapter.py 81 6 coverage-93%
voicevox_engine/core/core_initializer.py 60 30 coverage-50%
voicevox_engine/core/core_wrapper.py 228 160 coverage-30%
voicevox_engine/dev/init.py 0 0 coverage-100%
voicevox_engine/dev/core/init.py 0 0 coverage-100%
voicevox_engine/dev/core/mock.py 65 2 coverage-97%
voicevox_engine/dev/tts_engine/init.py 0 0 coverage-100%
voicevox_engine/dev/tts_engine/mock.py 28 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifest.py 36 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 11 0 coverage-100%
voicevox_engine/engine_manifest/init.py 0 0 coverage-100%
voicevox_engine/library_manager.py 92 4 coverage-96%
voicevox_engine/metas/Metas.py 36 0 coverage-100%
voicevox_engine/metas/MetasStore.py 28 1 coverage-96%
voicevox_engine/metas/init.py 0 0 coverage-100%
voicevox_engine/model.py 180 3 coverage-98%
voicevox_engine/morphing.py 72 4 coverage-94%
voicevox_engine/preset/Preset.py 13 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 82 2 coverage-98%
voicevox_engine/preset/init.py 0 0 coverage-100%
voicevox_engine/setting/Setting.py 9 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 20 0 coverage-100%
voicevox_engine/setting/init.py 0 0 coverage-100%
voicevox_engine/tts_pipeline/init.py 0 0 coverage-100%
voicevox_engine/tts_pipeline/kana_converter.py 88 1 coverage-99%
voicevox_engine/tts_pipeline/mora_mapping.py 7 0 coverage-100%
voicevox_engine/tts_pipeline/phoneme.py 34 0 coverage-100%
voicevox_engine/tts_pipeline/text_analyzer.py 146 6 coverage-96%
voicevox_engine/tts_pipeline/tts_engine.py 268 9 coverage-97%
voicevox_engine/user_dict/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/user_dict/user_dict.py 146 12 coverage-92%
voicevox_engine/utility/init.py 0 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_utility.py 6 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 13 0 coverage-100%
voicevox_engine/utility/path_utility.py 26 6 coverage-77%
voicevox_engine/utility/run_utility.py 10 7 coverage-30%
TOTAL 2521 521 coverage-79%

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

方針良いと思います!
ただ変更が一長一短になってしまっているので解決策をコメントしてみました!

.github/workflows/build.yml Outdated Show resolved Hide resolved
@tarepan
Copy link
Contributor Author

tarepan commented Mar 28, 2024

@Hiroshiba
全指摘箇所の反映・テストパスを確認しました。Re-review よろしくお願いします。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

ほぼLGTMです!!

せっかくなのでということでコメントをいっぱい書いてしまったのですが、ドキュメントは書かれていることが何より良いので、比較的サラッと流しめで捉えていただければと思います 🙇

.github/workflows/build-docs.yml Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/test-engine-container.yml Outdated Show resolved Hide resolved
.github/workflows/test-engine-container.yml Outdated Show resolved Hide resolved
.github/workflows/test-engine-package.yml Outdated Show resolved Hide resolved
@tarepan
Copy link
Contributor Author

tarepan commented Apr 15, 2024

@Hiroshiba
全指摘箇所の反映・テストパスを確認しました。Re-Re-review よろしくお願いします。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!

ものすごく見通しが良くなったと思います、ありがとうございます!
こちらでちょっと細かいところだけ調整させていただこうと思います!

.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/test-engine-package.yml Outdated Show resolved Hide resolved
.github/workflows/test-engine-package.yml Outdated Show resolved Hide resolved
@Hiroshiba Hiroshiba merged commit 6e98356 into VOICEVOX:master May 1, 2024
3 checks passed
@tarepan tarepan deleted the refactor/step_name branch May 1, 2024 10:52
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.

2 participants