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

chore: minor refactor #833

Merged
merged 3 commits into from
Sep 19, 2024
Merged

chore: minor refactor #833

merged 3 commits into from
Sep 19, 2024

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Sep 19, 2024

内容

単体でPRを出すには微妙な、細かいリファクタです。

主に不要なコードの削除と、嘘コメントの訂正です。diffの範囲が大きくなりそうなやつは見送りました。

関連 Issue

その他

Comment on lines -86 to -92
/*
* Cの関数として公開するための型や関数を定義するこれらの実装はvoicevox_core/publish.rsに定義してある対応する関数にある
* この関数ではvoicevox_core/publish.rsにある対応する関数の呼び出しと、その戻り値をCの形式に変換する処理のみとする
* これはC文脈の処理と実装をわけるためと、内部実装の変更がAPIに影響を与えにくくするためである
* voicevox_core/publish.rsにある対応する関数とはこのファイルに定義してある公開関数からvoicevoxプレフィックスを取り除いた名前の関数である
*/

Copy link
Member Author

Choose a reason for hiding this comment

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

代わりの文面が思い付かなかったので、役割を終えたということで丸ごと削除。

Copy link
Member Author

Choose a reason for hiding this comment

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

なんだったっけこのファイルの役割…? blameしてもよくわからない。

Copy link
Member

Choose a reason for hiding this comment

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

importできるかのチェック・・・・・・・・・・・・・・・・・・?
conftest側をimportしているのはちょっと理由わからないですが。。。。

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

いくつかコメント書きました!
もし追加で変更しても、軽微ならそのままマージ頂いても!

@qryxip qryxip merged commit c7c8cf1 into VOICEVOX:main Sep 19, 2024
31 checks passed
@qryxip qryxip deleted the chore-minor-refactor branch September 19, 2024 22:02
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