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

整理: 不使用の SupportedFeaturesInfo を削除 #1343

Merged
merged 2 commits into from
Jun 2, 2024

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented May 28, 2024

内容

voicevox_engine/model は API とエンジン内部で共有されるモデルの置き場である。
SupportedFeaturesInfo はここで 2 年前に定義された(#378)が、当初から現在まで不使用のクラスである。
よって削除しても影響がない。

このような背景から、SupportedFeaturesInfo の削除によるリファクタリングを提案します。

関連 Issue

無し

その他

本 PR の過程で、エンジン実装の中枢たる model に不使用クラスが 2 年以上存在し続けていたことがわかりました。
このことは、多種多様なモジュールから依存されるモジュール(いわゆる"神モジュール")はメンテが難しいことの証左です。
#1278 (comment) で議論されたとおり一定の有用性はありますが、各機能下の model.py へ分散して凝集度高く管理する方が賢明であることを示唆しています。

@tarepan tarepan requested a review from a team as a code owner May 28, 2024 13:47
@tarepan tarepan requested review from Hiroshiba and removed request for a team May 28, 2024 13:47
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!!

これは神モジュールというより、型定義と実装を分けてマージすると片方忘れることがあるということな気がしました。

voicevox_engine/model.py Show resolved Hide resolved
@Hiroshiba Hiroshiba merged commit 3b79fbc into VOICEVOX:master Jun 2, 2024
4 checks passed
@tarepan tarepan deleted the refactor/unused_model branch June 2, 2024 06:59
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