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.

Sign up for GitHub

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

Jump to bottom

fix!: writeFileSafely関数を非同期に変更し、テストを追加 #2502

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix!: writeFileSafely関数を非同期に変更し、テストを追加 #2502

wants to merge 2 commits into from
+42 −6

Conversation

Hiroshiba
Copy link
Member

@Hiroshiba Hiroshiba commented Jan 23, 2025 •
edited
Loading

内容

writeFileSafely関数をmove-file非依存にし、テストを追加しました。

を解決します。
詳細はこちら

関連 Issue

fix: #2501

@Hiroshiba Hiroshiba requested a review from a team as a code owner January 23, 2025 17:00
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Jan 23, 2025 •
edited
Loading

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ：b09ceb1

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Jan 23, 2025 •
edited
Loading

@madosuki さん、あと @sabonerune さん、もしよかったらレビューいただけると心強いです！！
想定外の処理を思いついた、とか！ 🙏

@sabonerune
Copy link
Contributor

sabonerune commented Jan 23, 2025 •
edited
Loading

@Hiroshiba
非同期になっていないけれど多分大丈夫?

fs.renameSync()のドキュメントにはPOSIXのrename(2)のドキュメントを見ろしか書いていません。
rename(2)には移動先がディレクトリ名の場合ディレクトリの中に書き込まれるとあります。

move-fileのmkdirはそれを防ぐため?

一方コールバック版のfs.rename()は移動先にディレクトリがあるとエラーになるそうです。

試したところfs.renameSync()はどうやらfs.rename()と同じように移動先にディレクトリがあるとエラーになるので多分大丈夫です。

@madosuki
Copy link
Contributor

概ね問題無さそうです。
後はsaboneruneさんと同じく、非同期になってないのは気になります。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@Hiroshiba @sabonerune @madosuki