MRG: adjust Signature::name() to return Option<String> instead of filename() and md5sum()
#3434
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## latest #3434 +/- ##
==========================================
- Coverage 86.44% 86.42% -0.03%
==========================================
Files 137 137
Lines 16102 16106 +4
Branches 2219 2219
==========================================
- Hits 13920 13919 -1
- Misses 1875 1880 +5
Partials 307 307
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -351,7 +351,15 @@ impl RevIndexOps for RevIndex { | |||
| .collection | |||
| .record_for_dataset(dataset_id) | |||
| .expect("dataset not found"); | |||
| Some((row.name().into(), size)) | |||
|
|
|||
| let mut name = row.name(); | |||
There was a problem hiding this comment.
This is ugly - is there an idiomatic way to do this??
There was a problem hiding this comment.
There was a problem hiding this comment.
maybe?
let name = [row.name(), row.filename(), row.md5()]
.into_iter()
.skip_while(|v| v.is_empty())
.next()
.unwrap(); // guaranteed to succeed because `md5` always exists
ctb
changed the title
Signature::name() return empty string if name is NoneSignature::name() return empty string if name is None
|
@luizirber @bluegenes ready for review! |
src/core/src/signature.rs
Outdated
| @@ -448,10 +448,8 @@ impl Signature { | |||
| pub fn name(&self) -> String { | |||
There was a problem hiding this comment.
since r0.18.0 is not released yet, do we want to make this
pub fn name(&self) -> Option<String> {
instead? This wil change calls around in other places, but seems to communicate better that the name exists or not instead of getting a .name() call that might return ""
There was a problem hiding this comment.
ooh, I like it! let me give it a try 🙇
ctb
changed the title
Signature::name() return empty string if name is NoneSignature::name() to return Option<String> instead of filename() and md5sum()
|
ready for review @luizirber ! |
This PR adjusts
Signature::name()to returnNonewhen no name is set, instead of returning firstfilename()or (if empty)md5sum(). It also addsname_str()which returns an empty string, to avoid too manyunwrap_orscattered throughout the codebase.Fixes #3441