Skip to content

Replace godirwalk with fs.WalkDir #23

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.

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

Merged
merged 2 commits into from
Aug 20, 2024
Merged

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Aug 20, 2024

This PR is a follow-up to #22 and replaces one lib requiring os file operations.

I measured the performance of godirwalk and fs.WalkDir on the Linux kernel source and there were no major performance differences.

See also: https://engineering.kablamo.com.au/posts/2021/quick-comparison-between-go-file-walk-implementations/

@mafredri mafredri self-assigned this Aug 20, 2024
@mafredri mafredri requested review from johnstcn and mtojek August 20, 2024 13:25
return filepath.SkipDir
}
return nil
filesystem.WalkDir(dir, func(path string, ent fs.DirEntry, err error) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

Review: I didn't add err handling here, surprised at how much err handling is missing but didn't feel like this PR is the right place to add it.

@mafredri mafredri force-pushed the mafredri/remove-godirwalk-dep branch from 2655b9d to 49af11a Compare August 20, 2024 13:48
Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

stamp 👍

@mafredri mafredri merged commit 96621b5 into main Aug 20, 2024
9 checks passed
@mafredri mafredri deleted the mafredri/remove-godirwalk-dep branch August 20, 2024 14:18
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.

3 participants