Crate evaluation for 2017-06-13: walkdir
For additional contribution opportunities, see the main libz blitz thread.
This post is a wiki. Feel free to edit it.
Links
- https://crates.io/crates/walkdir
- https://github.com/BurntSushi/walkdir
- https://docs.rs/walkdir
- https://air.mozilla.org/rust-libs-meeting-2017-06-13/
- https://github.com/BurntSushi/walkdir/issues/47
Needs your help!
Anything that is not checked off still needs your help! There is no need to sign up or ask for permission - follow any of these links and leave your thoughts:
Guidelines checklist
Legend
-
[y]
= guideline is adhered to, no work needed. -
[n]
= guideline may need work, see comments nearby -
[/]
= guideline not applicable to this crate
Checklist
This document is collaboratively editable. Pick a few of the guidelines, compare the walkdir
crate against them, and fill in the checklist with [y]
if the crate conforms to the guideline, [n]
if the crate does not conform, and [/]
if the guideline does not apply to this crate. Each guideline is explained in more detail here. If [n]
, please add a brief note on the following line with an explanation. For example:
- [n] Crate name is a palindrome (C-PALINDROME)
- walkdir backwards is ridklaw which is not the same as walkdir
Cookbook use case statements
Come up with ideas for nice introductory examples of using walkdir
, possibly in combination with other crates, that would be good to show in the Rust Cookbook. Please leave a comment in that issue with your ideas! You don’t necessarily have to write the example code yourself but PRs are always welcome.
API guideline updates
What lessons can we learn from walkdir
that will be broadly applicable to other crates? Please leave a comment in that issue with your ideas!
Discussion topics
Already stable
walkdir
is already stable, so we should try find non-breaking solutions, but @burntsushi isn’t against a 2.0
release if there’s enough in it.
Follow-up
- Publish
2.0
Links to types in std
There are a lot of references to types in std::fs
. Should these be links to the relevant section in the std
API docs? If so, where should they point?
Follow-up
- Link
std
references to https://doc.rust-lang.org/stable/std/ - Ping @steveklabnik about the
stable/std
path forstd
docs. Will this change?
Custom iterator adapters
WalkDirIterator::filter_entry
is not obviously better than into_iter().filter_map
.
filter_entry
behaves differently to filter
in a way that isn’t clear from the function signatures.
Are there expectations on what an Iterator adapter should look like?
- Predicate input as
Self::Item
or&Self::Item
:filter_entry
takes&DirEntry
instead ofResult
- Chainable with any other adapter:
filter_entry
is only available onWalkDirIter
Could we solve this with a new name/docs/examples:
- Maybe rename to
filter_walk
? It might be helpful to think of it as a driver for the walk that always comes beforefilter
, operating on an already determined set of entries - Example for use-cases that exploit the file structure (and may want to
filter_entry
) - Example for use-cases that ignore the file structure (and wouldn’t want to
filter_entry
)
Or make filter_entry
a method on WalkDir
instead of an adapter?
Follow-up
- Make
WalkDirIterator
methods inherent onIter
andIterFilterEntry
- Deprecate
WalkDirIterator
trait in favour of inherent methods - Remove
WalkDirIterator
trait in2.0
- Add doc example for
filter_entry
vsfilter
why choose one over the other
Should WalkDirIterator
be externally implementable?
The WalkDirIterator
is implemented by a few types internally, but doesn’t seem to have any value being implemented externally. Should it have a Sealed
supertype to signal that it’s not for implementing?
Follow-up
- Irrelevant since we’re removing the trait
Ensuring Send/Sync
Determining send/sync still hard. WalkDirOptions
is not Send
, which prevents WalkDir
and Iter
from being Send
. Maybe we could create assert_send!
, assert_sync!
macros and apply them to every type.
Should we strengthen recommendation to assert types are send/sync by default?
Follow-up
- Make
sort_by
functionSend + Sync
- Add
assert_send
/assert_sync
macros and test:WalkDir
Iter
IterFilterEntry
-
@KodrAus to open a Pre-RFC about asserting
Send
/Sync
instd
Error conditions for iterators returning results
Where should Iter::next
's Error conditions be documented if Iter::Item
is a Result
? On the impl of next? On the iterator itself?
Follow-up
- Add Errors section to
Iter
andIterFilterEntry
type docs - @KodrAus to file guideline issue about Error sections for trait methods returning errors
- @KodrAus to file issue about rust-doc calling out overridden trait docs more
Verified unwrapping
walkdir::Iter.next()
calls unwrap
on a Vec
that’s guaranteed not to panic by the block it’s in. Should this be expect
with a comment describing why it can’t fail?
Could any of these be refactored to avoid the unwrap
altogether? All methods calling unwrap
that are validated:
<Iter as Iterator>::next
Iter::get_deferred_dir
Iter::push
Should there be a guideline for this?
Follow-up
- Change
unwrap
toexpect
and document why they won’t panic - Open guidelines issue about
unwrap
vsexpect
Re-exporting unstable is_same_file
Should the is_same_file
function be re-exported from the unstable same_file
crate? It isn’t mentioned in docs or used in examples. Looks like it was pulled into a new crate post 1.0
and re-exported to prevent a breaking change.
Follow-up
- Deprecate re-export in favour of
is_same_file
crate - Remove re-export in
2.0
Sorting entries
Could sorter
support more metadata to compare like size, last modified? It should at least use OsStr
instead of &OsString
.
Follow-up
- Open issue to support sorting by more file metadata (might not be feasible)
- At least change
&OsString
args toOsStr
Platform-specific methods
Raised during the meeting that DirEntry::ino
is a unix only method, but that isn’t reflected in the docs.
Follow-up
- Add
unix::DirEntryExt
trait and addino
method there - Remove inherent
ino
method fromDirEntry
- Document that
DirEntryExt
is unix only
Crate issues
[details=Summary]- Add Error documentation sections for
- is_same_file
-
Iter
/Iter::next
- probably should be documented in both places? See discussion -
IterFilterEntry
/IterFilterEntry::next
- DirEntry::metadata
- docs: Indicate that DirEntry::ino is Unix only
- docs: remove unwrap from
- examples in root
- examples on
WalkDir
- examples on
WalkDirIterator.skip_current_dir
andfilter_entry
- docs: clarify differences between
walkdir::DirEntry
andstd::fs::DirEntry
more- add examples for
DirEntry
methods - demonstrate differences with
std::fs::DirEntry
in method examples
- add examples for
- docs: add example for
contents_first
- docs: add links for:
-
filter_entry
in root skip hidden files and directories efficiently on unix example -
path
,file_name
andfollow_links
inDirEntry
-
WalkDir::new
inDirEntry::path
-
follow_links
inDirEntry::path_is_symbolic
-
follow_links
inDirEntry::metadata
-
follow_links
inDirEntry::file_type
-
WalkDir
inIter
-
WalkDir::min_depth
inIterFilterEntry
-
WalkDir::max_depth
inIterFilterEntry
-
WalkDir::filter_entry
inWalkDir
-
WalkDir::min_depth
inWalkDir
-
WalkDirIterator::filter_entry
inWalkDirIterator::skip_current_dir
-
WalkDirIterator::skip_current_dir
inWalkDirIterator::filter_entry
-
WalkDir::min_depth
inWalkDirIterator::filter_entry
-
WalkDir::max_depth
inWalkDirIterator::filter_entry
-
- docs: Add a note that
IterFilterEntry
is created by callingIter::filter_entry
- docs: Correct inaccuracies in
WalkDir
root docs: ifcontents_first
istrue
then directories aren’t emitted before their contents. The order isn’t unspecified if asort_by
predicate is given - Add
html_root_url
attribute - Add Debug to Iter, IterFilterEntry, WalkDir
- Add CI for OSX (?) and crate badges for tier 1 platforms
- Add categories metadata to Cargo.toml
- Add
Debug
impls forIter
IterFieldEntry
WalkDir
- Rename
Iter
toIntoIter
- Rename
IterFilterEntry
toFilterEntry
[/details]
Evaluation task assignments
- [ ] @KodrAus
assert_send
/assert_sync
Pre-RFC - [ ] @KodrAus follow up stability of
/stable/std
path tostd
docs - [ ] @KodrAus follow up rustdoc calling out overridden trait method docs
How are we tracking?
Pre-review checklist
Do these before the libs team review.
- [x] Create evaluation thread based on this template
- [x] Work with author and issue tracker to identify known blockers
- [x] Compare crate to guidelines by filling in checklist
- [x] Record other questions and notes about crate
- [x] Draft several use case statements to serve as cookbook examples
- [x] Record recommendations for updated guidelines
Post-review checklist
Do these after the libs team review.
- [x] Publish libs team meeting video
- [x] Create new issues and tracking issue on crate’s issue tracker
- [ ] Solicit use cases for cookbook examples related to the crate
- [ ] File issues to implement cookbook examples
- [ ] File issues to update guidelines
- [ ] Post all approachable issues to TWiR call for participation thread
- [ ] Update links in coordination thread