-
Notifications
You must be signed in to change notification settings - Fork 32
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
Label deletion #68
Label deletion #68
Conversation
Codecov Report
@@ Coverage Diff @@
## master #68 +/- ##
==========================================
- Coverage 72.09% 72.04% -0.05%
==========================================
Files 32 32
Lines 12175 12248 +73
Branches 2181 2205 +24
==========================================
+ Hits 8777 8824 +47
+ Misses 1090 1079 -11
- Partials 2308 2345 +37
Continue to review full report at Codecov.
|
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.
Looks good. Agree with you that locking on deletion is not necessary. Also the code in general looks way better with the async trait and is easier to read. No weird stuff as far as I can see in the added deletion logic.
@@ -288,83 +290,71 @@ async fn get_label_from_exclusive_locked_file<P: Into<PathBuf>>( | |||
Ok((label, file)) | |||
} | |||
|
|||
#[async_trait] |
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.
Nice 😍
let mut p = self.path.clone(); | ||
p.push(format!("{}.label", name)); | ||
|
||
// We're not locking here to remove the file. The assumption |
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.
Agree
Code for deleting labels.
Closes #8, and contributes to #56.