Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

Split Entry into two different enums to remove unnecessary expects #272

Merged
merged 1 commit into from
Jul 31, 2020

Conversation

c410-f3r
Copy link
Contributor

Second take on #200

Entry is now composed of Bucket(...) and Metadata(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.

@c410-f3r c410-f3r force-pushed the unwraps branch 2 times, most recently from b95deaf to aa9fe29 Compare July 30, 2020 13:39
@koivunej
Copy link
Collaborator

hello @c410-f3r and thanks for your continued work with #200. this is looking like a good step forward. could you separate the walk.rs reordering into a separate commit or keep the old ordering to make the reviewing a bit easier?

@c410-f3r c410-f3r force-pushed the unwraps branch 2 times, most recently from c0b8743 to 6ca7533 Compare July 31, 2020 11:39
Copy link
Collaborator

@koivunej koivunej left a 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?

Comment on lines 492 to 511
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),
}
}
Copy link
Collaborator

@koivunej koivunej Jul 31, 2020

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

@c410-f3r
Copy link
Contributor Author

@koivunej Thanks

That is all regarding removing expect(""). My current local changes are now solely related to avoid double matching but it will take some time to complete.

@koivunej
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 31, 2020

Build succeeded:

@bors bors bot merged commit c6b68cc into rs-ipfs:master Jul 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants