fix: voicevox-cpu等の使用しないディレクトリが作成される問題を修正 #1620
Conversation
electron-logをv5へ
app.getPath("userData")で作成されてしまうディレクトリを一度削除するようにした
一部の===undefinedを修正
| ipcRenderer.send("__ELECTRON_LOG__", { | ||
| data: [...params], | ||
| level: "error", | ||
| }); |
There was a problem hiding this comment.
https://github.com/megahertz/electron-log/blob/master/README.md#renderer-process
import log from 'electron-log/renderer';
log.info('Log from the renderer process');わざわざ手動で呼び出す必要はないような感じがします、何か問題とかあったらコメントがあると判りやすいかと。
There was a problem hiding this comment.
以前のバージョンでもプリロードでelectron-logをインポートしていなかったのでそれに倣った感じです。
また、具体的な問題は発生していませんが
- レンダラープロセスの状態をブラウザ版と可能な限り変えない方がいいと感じた。
log.initialize({ preload: true });で呼び出すとuserDataディレクトリにelectron-log-preload.jsが作成されるという挙動に怪しさを感じた。
というのもあります。
There was a problem hiding this comment.
ブラウザ版の考慮はすごい納得感ありました。
ここでelectron-logをimportするのはまあOKだと思いますが、userDataにjsファイルが作られるのはちょっとよくわかんないですね･･･。
まあドキュメントでも案内されてる方法ですし、この形が良いのかなと思いました。
https://github.com/megahertz/electron-log/blob/19bb2a1ab5e12eb985f87cae058d54f6970c7d49/docs/initialize.md#using-ipc-directly
コメントは１箇所あると良さそうですね！
ここのやりとりへのリンクをコメントする感じとか。
Hiroshiba
left a comment
Hiroshiba
left a comment
There was a problem hiding this comment.
LGTM！！
･･･なのですが、正直ちょっとしばらく様子見したいなという気持ちもちょっとあります 😇
メジャーバージョンアプデなので、なんか大きくガラッと仕様変わったらやだな～くらいの気持ちです。
･･･２週間待機とかでどうでしょう！
| // バージョン0.14より前の設定ファイルの保存場所 | ||
| const beforeUserDataDir = app.getPath("userData"); // マイグレーション用 | ||
|
|
||
| // app.getPath("userData")を呼ぶとディレクトリが作成されてしまうため空なら削除する。 |
| ipcRenderer.send("__ELECTRON_LOG__", { | ||
| data: [...params], | ||
| level: "error", | ||
| }); |
There was a problem hiding this comment.
ブラウザ版の考慮はすごい納得感ありました。
ここでelectron-logをimportするのはまあOKだと思いますが、userDataにjsファイルが作られるのはちょっとよくわかんないですね･･･。
まあドキュメントでも案内されてる方法ですし、この形が良いのかなと思いました。
https://github.com/megahertz/electron-log/blob/19bb2a1ab5e12eb985f87cae058d54f6970c7d49/docs/initialize.md#using-ipc-directly
コメントは１箇所あると良さそうですね！
ここのやりとりへのリンクをコメントする感じとか。
Co-authored-by: Hiroshiba <[email protected]>
スタイル修正
|
マージします！ |
| try { | ||
| fs.rmdirSync(beforeUserDataDir, { recursive: false }); | ||
| } catch (e) { | ||
| const err = e as NodeJS.ErrnoException; | ||
| if (err?.code !== "ENOTEMPTY") { | ||
| log.error(err); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@Hiroshiba マージした後ですみません。
electron-logの初期化や設定前にlog.error(err);を呼びだしてしまっているのでNOTENPTY以外のエラーが起きた時beforeUserDataDirの方にログを作成してしまうことに気が付きました。
There was a problem hiding this comment.
おっとなるほどです！！気づいてくださってありがとうございます！
修正プルリクエストをお願いしても良いでしょうか？
（こちらで良い感じに直しておくのももちろんOKです！）
3 participants
内容
electron-logがインポートすると自動的にログディレクトリを作成してしまうこと、初期状態でapp.getPath("userData")を実行すると自動的にディレクトリを作成してしまうことが原因でした。このため
electron-logを問題が修正された5.0.0へ更新、beforeUserDataDirを取得した後、そのディレクトリを削除することで問題を解決します。関連 Issue
その他
ref electron-log
portManager.tsの!== undefinedを修正vvppManager.tsの=== nullを修正