Skip to content

Conversation

@linbingquan
Copy link
Contributor

Rust 1.85.1 releases: https://github.com/rust-lang/rust/releases/tag/1.85.1

Rust 1.85.1 releases log has std::fs::rename fix PR

Deno.renameSync API error with CI test, Perhaps it is caused by this PR, when I try to upgrade Rust 1.86.0 or Rust 1.87.0

Here use CI to test.

Comment on lines 255 to 262
// see https://github.com/rust-lang/rust/pull/137528
// assertThrows(
// () => {
// Deno.renameSync(olddir, file);
// },
// Error,
// "The directory name is invalid",
// );
Copy link
Member

@dsherret dsherret May 23, 2025

Choose a reason for hiding this comment

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

Instead of commenting this out, maybe we should revert it back to what it was before? Looks like this test used to be different before Rust 1.85.

ee4c14a#diff-b7856c2634c543995bc8dab9f355ac08fa54e894d1012ebaa349d35d4b64a3feL271

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of commenting this out, maybe we should revert it back to what it was before? Looks like this test used to be different before Rust 1.85.

Thank you for your reply. I compared the previous code and now the existing test cases are duplicated.

// should succeed on Windows
Deno.renameSync(olddir, emptydir);

Copy link
Member

Choose a reason for hiding this comment

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

Ok, maybe let’s remove this commented out code? It seems unnecessary to keep around now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, maybe let’s remove this commented out code? It seems unnecessary to keep around now.

Done, and I was rebase onto main

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@dsherret dsherret enabled auto-merge (squash) May 24, 2025 03:32
@dsherret dsherret merged commit ff078dc into denoland:main May 24, 2025
18 checks passed
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