-
Notifications
You must be signed in to change notification settings - Fork 310
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
IPC周りの呼び出しをdot記法で書けるように #2240
IPC周りの呼び出しをdot記法で書けるように #2240
Conversation
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.
プルリクエストありがとうございます!!
もしかしたらより使い勝手が良いというか、メンテナンスしやすい形があるかもですが、とりあえず初学者の人がコードをコードジャンプで追えるようになってとても良いと思います!!
(まあ参照に移動しないといけないのでちょっと難度は高いのですが、)
@Segu-g さん
すみません!
もしお時間あれば型周りに関してアドバイスいただけると心強いです!!
受信側をオブジェクトで書くことで実装へコードジャンプが可能になりました。 オブジェクトで書くようになった影響で未実装(未使用)の項目が見つかったので、それをどうするかが決まるまではDraftにしておこうと思います。 |
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.
型を確認しました!
mainとrendererのどちらからもコードジャンプできていて良いと思います!
また、この辺りのコードが置き換え忘れているようです!
https://github.com/VOICEVOX/voicevox/pull/2240/files#diff-d2475d73ba5acb680e9dee5e4253a9bc70d470ed5835750c9832f21074090cd4R460-R476
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です!!
実装ありがとうございます!!
型はもっと詰められそうかもですが(registerIpcMainHandle
に渡すものがちゃんとVOICEVOXが想定するIPC通信用のオブジェクトになってるかなど)、まあこだわり過ぎなくても良いかもですね!
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!!!!
コードジャンプができるようになって初見の方が迷うポイントが1つ減ったと思います!!
ありがとうございます!!!!!
ちょっと話変わるんですが、チラッと出ていたレンダラー側を「フロントエンド」と呼んでIPC通信周りのコードを整理していくタスク、ちょっと興味あったりされませんか!
多分この辺り一番詳しいのMTさんなので、お任せできれば心強いな~~~~と思った次第です!!
ただちょっとややこしいので、何か他に興味があったりしたら全然大丈夫です!
もしご興味あればissueの方に質問or提案コメントなんかをいただけると!!!
本当にご興味あればぐらいの気持ちです 🙏
まあ今この辺りの知識にフレッシュなので勢いあるうちにやっときたい、くらい!
あ、 @Segu-g さんのrequestedが未解決に残ってるのでマージは保留ということで! |
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!
内容
#2088 の実現方法を参考に、IPC周りの呼び出しをdot記法で書けるようにしてみました。
関連 Issue
スクリーンショット・動画など
その他