-
Notifications
You must be signed in to change notification settings - Fork 166
Split Entry into two different enums to remove unnecessary expects #272
Conversation
b95deaf
to
aa9fe29
Compare
c0b8743
to
6ca7533
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thanks for reordering, much smaller changeset.
Is this good to merge, as in, do you have pending pushes?
pub fn as_entry(&self) -> Entry<'_> { | ||
use InnerKind::*; | ||
match &self.kind { | ||
RootDirectory(cid) | BucketAtRoot(cid) => { | ||
Entry::RootDirectory(cid, &self.path, &self.metadata) | ||
InnerKind::Bucket(cid) => Entry::Bucket(cid, &self.path), | ||
InnerKind::Directory(cid) => { | ||
Entry::Metadata(MetadataEntry::Directory(cid, &self.path, &self.metadata)) | ||
} | ||
InnerKind::File(cid, _, sz) => { | ||
Entry::Metadata(MetadataEntry::File(cid, &self.path, &self.metadata, *sz)) | ||
} | ||
InnerKind::RootBucket(cid) => { | ||
Entry::Metadata(MetadataEntry::Directory(cid, &self.path, &self.metadata)) | ||
} | ||
InnerKind::RootDirectory(cid) | InnerKind::BucketAtRoot(cid) => Entry::Metadata( | ||
MetadataEntry::RootDirectory(cid, &self.path, &self.metadata), | ||
), | ||
InnerKind::Symlink(cid) => { | ||
Entry::Metadata(MetadataEntry::Symlink(cid, &self.path, &self.metadata)) | ||
} | ||
RootBucket(cid) => Entry::Directory(cid, &self.path, &self.metadata), | ||
Bucket(cid) => Entry::Bucket(cid, &self.path), | ||
Directory(cid) => Entry::Directory(cid, &self.path, &self.metadata), | ||
File(cid, _, sz) => Entry::File(cid, &self.path, &self.metadata, *sz), | ||
Symlink(cid) => Entry::Symlink(cid, &self.path, &self.metadata), | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In hindsight this seems "really obvious" change, great idea!
Thinking of this naming or meaningwise the difference between the cases might be more on the Entry::Metadata
meaning "is standalone entry [which might have non-empty metadata]" and Entry::Bucket
meaning "is continuation [or 'just more links']" but as I think the doc comments on Entry
variants already cover this detail nicely so no need to change this right now.
EDIT: fmt
@koivunej Thanks That is all regarding removing |
bors r+ |
Build succeeded: |
Second take on #200
Entry
is now composed ofBucket(...)
andMetadata(MetadataEntry { ... })
to separate things that have and doesn't have metadata, which avoids returning optional values derived from a single entry-point.Next PR will address the newly introduced double matching while iterating over
continue_walk
but it will require deeper logical changes.