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

add: リリースノートに機械可読な<table>を置く #43

Merged
merged 6 commits into from
Aug 3, 2024

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Jul 29, 2024

内容

VOICEVOX/voicevox_core#810 のための変更の一つです。
(VOICEVOX/voicevox_core#810 から依存されているのは、このPRそのものではなく #44 です)

リリースするONNX Runtimeについて機械可読な<table>を置き、CORE側のダウンローダーから認識できるようにします。

関連 Issue

ref VOICEVOX/voicevox_core#810

スクリーンショット・動画など

その他

@qryxip
Copy link
Member Author

qryxip commented Jul 29, 2024

懸念: https://discord.com/channels/879570910208733277/893889888208977960/1267039799068590144

CORE側ではcomrakとscraperでCargo.lockが630行膨らむし、やってることの絵面もやはりよろしくはない。

別案の一つとしては手書きのJSONをリポジトリに置いて整合性はテストで担保するというのもあるが、ビルド時に情報を作るという流れにも利点はある。となるとリリースノート上の<table>でもよい...?

@Hiroshiba
Copy link
Member

コアのダウンローダー側見てみたんですが、リリースノートをパースする形で全然いいんじゃないかなと思いました!
ってことでレビューします!!

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.yml Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
Comment on lines 537 to 539
## XCFramework

<table id="voicevox-onnxruntime-specs-v1format-v$onnxruntime_version_hyphenated-xcframeworks">
Copy link
Member

Choose a reason for hiding this comment

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

実は分ける理由ないかも・・・?

・・・うーん、難しいですね。。。
まあ別に深く考えず、何でもいいかな・・・。

あ、コア側からダウンロードするつもりならxcframeworksだけ分ける形は後々大変になるかもです。
何かとxcframeworksをくっつける可能性もあると思うので。

コアからxcframeworksもダウンロードするならdylibsとxcframeworksはわけない方が良いか、あるいはdylibsとothersとかにしておくとまだ変更に強そう。
xcframeworksをコアのダウンローダーからダウンロードしないのであれば何でも良さそう。(idをつけない方が良さそう・・・?)

まあたぶんdylibsとxcframeworksは分けときたいってことだと思うので、将来的なこと考えるとこっちをothersにしとくのが丸い・・・かなぁ。
増えるとしたらmavenとかunitypackageとかnpmとかですかねぇ。

・・・npmは増えそう!!だけどOS項目いらなそう!!
うーーーーーん。dylibs/xcframeworksでも別に良い気もしてきました。
わからん!

qryxip added a commit to qryxip/onnxruntime-builder that referenced this pull request Aug 1, 2024
qryxip added a commit to qryxip/onnxruntime-builder that referenced this pull request Aug 1, 2024
@qryxip
Copy link
Member Author

qryxip commented Aug 2, 2024

VOICEVOX/voicevox_core#810 (comment)に対応するものです。
0c25909 (#43)
0c31816 (#43)

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

@Hiroshiba Hiroshiba merged commit bc8e892 into VOICEVOX:main Aug 3, 2024
1 check passed
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