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

追加: VVM ファイル詳細 docs #760

Merged
merged 3 commits into from
Mar 29, 2024
Merged

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Mar 7, 2024

内容

概要: VVM ファイル docs にファイル詳細(定義・内部構造・例)を追加した

VVM の詳細を記述する docs が存在しないため、既存 docs への加筆により情報を補足した。
これにより、v0.16 リリース issue (#545) の「[ ] VVMのマニフェストのデータ構造に関してメモをどこかに書く」が解決される。

関連 Issue

ref #545

@qryxip
Copy link
Member

qryxip commented Mar 10, 2024

個人的な意見ですが、リポジトリ内のドキュメントを利用者向けと貢献者向けに分類するとき、VVMの内部構造については後者に分類されると思っています。

というのも利用者向けとしては我々的に保証はしたくない部分が結構あると思っていて、例えばZIPファイルとしてのファイル構造は、今後ソング用VVMが入ってくるとonnxファイルのセットが異なってきます (#761 で考えているように、{talk, singing_teacher, frame_decode}²で2³通りの構造を考えることになるかなと思っています)。またmanifest.jsonの形式も変える予定です。

(edit) (現時点ではまだ無い)vvm_format_versionの存在ならユーザーに教えてもいいかもしれません (unzip -pjqを使って読むコマンド付きで)。

なのでdocs/vvm.mdにドキュメントを書いてREADMEからリンクする、というのが良いのかどうかがわからないです。

@Hiroshiba 「VVMのマニフェストのデータ構造に関してメモをどこかに書く」というの、私はあまりよく覚えていないのですがどんな動機でしたっけ?

@Hiroshiba
Copy link
Member

Hiroshiba commented Mar 18, 2024

@qryxip 貢献者向けですね!

このコメントを見るに

開発者向けに、VVMのマニフェストの構造に関してのメモがどこかにあると良いなと感じました。

とのことでした。

だから利用者向けのとこからはリンクを張らず、貢献者向けのとこからリンクをはるのに賛成です!

@tarepan
Copy link
Contributor Author

tarepan commented Mar 18, 2024

リポジトリ内のドキュメントを利用者向けと貢献者向けに分類

この2分類の場合、@qryxip さんと @Hiroshiba さんに同意です。

ただ、(ENGINE でも似た議論があったのですが、)実際のリポジトリ利用者は 3 種類います。

  • 利用者
  • 開発者(clone / OSS forkする人)
  • 貢献者

です。

CORE の README.mdユーザーガイド節で別ページリンクを貼っています。
なのでそれ以降の記述は「開発者」「貢献者」向けとなるはずです。
この2者向けであれば、むしろ DX 向上のために VVM 詳細をリンクする方が好ましいと考えます。(CORE fork で VVM の理解に苦労したためこの PR を建てました。)
今回リンクを追加した 環境構築 節は ユーザーガイド 節以後に配置されているので、上記の条件に当てはまると考えます。

よって本 PR の箇所にリンクを張る方向性を支持します。
お二人のご意見を伺えれば幸いです。

@Hiroshiba
Copy link
Member

たしかにです!

となるとREADMEのだいぶ下の方で「VVMの内部構造」みたいなサブタイトルを付けつつ案内して、これはユーザー向けに保証してるものではないと案内するとかでしょうか。
あるいは、vvmを読み込む部分のコードにドキュメントのパスを書くのが良い気がしました!(こっちのが合ってそう)

一旦READMEなどからはリンクを張らず、あとでしかるべきところを探して足すとかもありだと思います。

@tarepan
Copy link
Contributor Author

tarepan commented Mar 21, 2024

vvmを読み込む部分のコードにドキュメントのパスを書く

👍
良いアイデアに感じました。
README.md を読む時点では VVM のこと知らない開発者/貢献者が大多数なので、それを知る段階の1つであるコードにコメントとしてドキュメントリンクを書き記すのは有用そうです。

@qryxip さんの意見を伺ったうえで判断したいと思います。

@qryxip
Copy link
Member

qryxip commented Mar 23, 2024

少なくとも現状からの一歩としては最善だと思いました。それでいきましょう。

(そろそろドキュメントをガンガン改善していきたいですね)

@tarepan
Copy link
Contributor Author

tarepan commented Mar 23, 2024

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

@tarepan tarepan requested a review from qryxip March 25, 2024 12:41
Copy link
Member

@qryxip qryxip left a comment

Choose a reason for hiding this comment

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

LGTM!

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!!

docs/vvm.md Show resolved Hide resolved
@Hiroshiba Hiroshiba merged commit 4db625d into VOICEVOX:main Mar 29, 2024
31 checks passed
@tarepan tarepan deleted the add/vvm_docs branch March 29, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants