Skip to content

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented May 18, 2023

内容

cbindgenが生成するvoicevox_core.hをこのリポジトリに追加します。

関連 Issue

Resolves #488.

その他

Copy link
Member

@sevenc-nanashi sevenc-nanashi 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_core.hをlinguist-generatedタグつけてもいいかと思いましたが、API変更のdiffとしても見れるのでない方がよさそう)

@PickledChair
Copy link
Member

PickledChair commented May 19, 2023

ヘッダファイルの diff が検出しやすくなるので良い変更だと思いました!

このままでも問題ないと思うのですが、ちょっと提案があります。今後 crates/voicevox_core_c_api/include/voicevox_core.h がある想定になると思うので、以下のステップを削除して、

- name: generate voicevox_core.h
shell: bash
run: cbindgen --crate voicevox_core_c_api -o ./voicevox_core.h

以下のコピー元を crates/voicevox_core_c_api/include/voicevox_core.h に書き換えても良いかもしれないと思いました。

cp -v voicevox_core.h "artifact/${{ env.ASSET_NAME }}"

(大して意味がなさそうでしたらこのままでも良いです。あるいは、何か懸念がありそうでしたらご指摘お願いします)

@qryxip
Copy link
Member Author

qryxip commented May 19, 2023

忘れてました。build_and_deployからcbindgenは今抜きました。

@qryxip
Copy link
Member Author

qryxip commented May 19, 2023

(voicevox_core.hをlinguist-generatedタグつけてもいいかと思いましたが、API変更のdiffとしても見れるのでない方がよさそう)

こっちも単純に忘れてました。.lockファイルと同じく、クリックしたら普通にdiffは見れるので入れた方がいいと思いました。

Copy link
Member

@PickledChair PickledChair left a comment

Choose a reason for hiding this comment

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

LGTM! 大丈夫だと思うのでマージします

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.

cbindgenが生成するvoicevox_core.hをこのリポジトリに追加する
3 participants