Skip to content

Delete empty dirs in baseline-accept task#2924

Open
jakebailey wants to merge 1 commit intomainfrom
jabaile/baseline-accept-delete-empty
Open

Delete empty dirs in baseline-accept task#2924
jakebailey wants to merge 1 commit intomainfrom
jabaile/baseline-accept-delete-empty

Conversation

@jakebailey
Copy link
Member

The empty dirs are pretty annoying sometimes when comparing. Just have the accept task delete them.

Copilot AI review requested due to automatic review settings February 27, 2026 00:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the repo’s baseline-accept hereby task to clean up empty baseline directories left behind after accepting deletions, reducing noise when comparing baseline trees.

Changes:

  • Added a helper to walk upward and remove empty directories after baseline deletions.
  • Extended baselineAcceptTask to run the empty-directory cleanup for both local and reference baseline trees.

let current = path.resolve(dir);
while (current !== resolvedRoot) {
try {
await fs.promises.rmdir(current);
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

fs.promises.rmdir() is deprecated in modern Node (repo targets Node >=20) and may start emitting warnings or be removed in future. Prefer fs.promises.rm(current, { recursive: false, ... }) (and consider using the same Windows retry behavior as rimraf) to remove empty directories without relying on rmdir.

Suggested change
await fs.promises.rmdir(current);
await fs.promises.rm(current, { recursive: false });

Copilot uses AI. Check for mistakes.
Comment on lines +860 to +872
async function removeEmptyDirsUp(dir, root) {
const resolvedRoot = path.resolve(root);
let current = path.resolve(dir);
while (current !== resolvedRoot) {
try {
await fs.promises.rmdir(current);
}
catch {
// Not empty or doesn't exist; stop walking up.
break;
}
current = path.dirname(current);
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

removeEmptyDirsUp assumes dir is within root, but it doesn't enforce that. If an unexpected path outside root is passed in (e.g. due to a bad glob result or future refactor), this could walk upward deleting unrelated empty directories until an error stops it. Consider guarding up front (e.g. ensure path.relative(resolvedRoot, current) doesn’t start with .. / isn’t absolute) and stopping when current leaves root or reaches filesystem root.

Copilot uses AI. Check for mistakes.
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