Copilot commented on code in PR #841:
URL:
https://github.com/apache/skywalking-banyandb/pull/841#discussion_r2502490550
##########
banyand/backup/restore.go:
##########
@@ -253,10 +253,11 @@ func cleanEmptyDirs(dir, stopDir string) {
if dir == stopDir || dir == "." {
break
}
- entries, err := os.ReadDir(dir)
- if err != nil || len(entries) > 0 {
+ files, err := getAllFiles(dir)
+ if err != nil || len(files) > 0 {
Review Comment:
The replacement of `os.ReadDir(dir)` with `getAllFiles(dir)` changes the
behavior significantly. `os.ReadDir` checks only the immediate directory for
entries (both files and subdirectories), while `getAllFiles` recursively walks
the entire tree and returns only files. This means:
1. If `dir` has empty subdirectories but no files, the old code would keep
it (len(entries) > 0), but the new code would remove it (len(files) == 0).
2. The function now recursively checks the entire tree instead of just the
immediate directory.
This could lead to unintended deletion of directories that contain empty
subdirectories. If this behavior change is intentional, it should be clearly
documented. Otherwise, consider using `os.ReadDir` to maintain the original
behavior of checking only the immediate directory.
```suggestion
entries, err := os.ReadDir(dir)
if err != nil || len(entries) > 0 {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
