-
Notifications
You must be signed in to change notification settings - Fork 122
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
feat!: VoiceModelFile::close
後もid
とmetas
へのアクセスを保証
#937
feat!: VoiceModelFile::close
後もid
とmetas
へのアクセスを保証
#937
Conversation
VoiceModelFile::close
後もid
とmetas
へのアクセスを保証VoiceModelFile::close
後もid
とmetas
へのアクセスを保証
VoiceModelFile::close
後もid
とmetas
へのアクセスを保証VoiceModelFile::close
後もid
とmetas
へのアクセスを保証
8429882
to
99530cc
Compare
99530cc
to
c8afcda
Compare
VoiceModelFile::close
後もid
とmetas
へのアクセスを保証VoiceModelFile::close
後もid
とmetas
へのアクセスを保証
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちょっと若干迷っていて、voicevox_voice_model_file_delete
一本に絞ってもよいかもしれません。id
とか知りたきゃ先に取っておけみたいな感じで。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
考えたのですが、voicevox_voice_model_file_delete
一本にしてしまいました。これによりC APIの変更は「voicevox_voice_model_file_close
をdelete
に改名」ということのみになります。理由としてはC API経由でバインディングを作る人とかに混乱をもたらすだろうと思ったからです。あと実装面でも(既にご覧になったと思いますが)小さくない複雑性をもたらすので、少なくとも今は要らないかなと思った次第です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちょっとわかってませんが、LGTM…?です!
そもそもopenしてないとid見れないんでしたっけ。
だったらちょっとだけ不便かもですね…!
いつでもid取得できる形にできればcloseの返り値はいらなくなりそう。
(closeでidとかが帰るのはかなり珍しそうな直感。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
とりあえず再掲
そもそもopenしてないとid見れないんでしたっけ。
だったらちょっとだけ不便かもですね…!いつでもid取得できる形にできればcloseの返り値はいらなくなりそう。
ちょっと理解しきれてないかもなのですが、それをやっているのがPythonとJavaで、Rustではムーブセマンティクスの概念に沿う形がいいかなと思ってます。このPRのdocs/guide/dev/api-design.mdに書いたみたいに。 |
あ、ほんとですね! openするとインスタンスが帰ってくるようになってたので問題なかった! |
内容
VoiceModelFile::close
の返り値を()
から(VoiceModelId, Vec<SpeakerMeta>)
にvoicevox_voice_model_file_close
とvoicevox_voice_model_file_delete
の二つを提供するようにvoicevox_voice_model_file_close
をvoicevox_voice_model_file_delete
に改名__(a)exit__
後もid
とmetas
にアクセス可能であることをドキュメント上で約束関連 Issue
その他