plumbing/storer: add RemoveReference #269
Conversation
| return nil | ||
| } | ||
|
|
||
| if err := s.Err(); err != nil { |
There was a problem hiding this comment.
I think this check must go before 350, so in case of error and not found we actually return the error instead of nil.
| return err | ||
| } | ||
|
|
||
| backupPackedRefs := packedRefsPath + ".bkp" |
There was a problem hiding this comment.
This function is already too long.
This last part is a very good candidate to be moved to a function called safeRename() or something similar. A good name will also help the reader to tell what 3 consecutive renames do :).
There was a problem hiding this comment.
I've just removed this and used billy's Rename directly. Doing our own SafeRename external to billy gets messy really fast (since guarantees depend both on operating system, file system and billy Filesystem implementation). So let's rely on billy implementations to provide guarantees as strong as they can on their own.
Codecov Report
@@ Coverage Diff @@
## master #269 +/- ##
=========================================
Coverage ? 76.38%
=========================================
Files ? 111
Lines ? 7605
Branches ? 0
=========================================
Hits ? 5809
Misses ? 1161
Partials ? 635
Continue to review full report at Codecov.
|
| @@ -18,6 +18,7 @@ type ReferenceStorer interface { | |||
| SetReference(*plumbing.Reference) error | |||
| Reference(plumbing.ReferenceName) (*plumbing.Reference, error) | |||
| IterReferences() (ReferenceIter, error) | |||
| RemoveReference(plumbing.ReferenceName) error | |||
There was a problem hiding this comment.
Some documentation indicating at least that could not be thread safe?
There was a problem hiding this comment.
Being thread-safe or not is implementation-dependent. So far, none of our public implementations is thread-safe for any write operation.
No description provided.