Conversation
siva/library.go
Outdated
| // not provided or already in metadata. | ||
| DontGenerateID bool | ||
| // Metadata flags if the library must use stored metadata. | ||
| StoredMetadata bool |
There was a problem hiding this comment.
Loading metadata should be the default behavior. I would enforce it for now and if needed we can add a DisableMetadata option when we find the use case.
siva/metadata.go
Outdated
| type Version struct { | ||
| // Offset is a position in the siva file. | ||
| // Bookmark represents a valid siva file point to read from. | ||
| type Bookmark struct { |
There was a problem hiding this comment.
What's the rationale for changing Version to Bookmark? It feels more natural that calling .Version() returns a Version and not a Bookmark.
155dd11 to
782580c
Compare
|
@jfontan I did the changes:
|
siva/library.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| if metadata != nil { |
There was a problem hiding this comment.
Shouldn't be if metadata != nil && id == "" {? There's the case where you provide an ID but there's metadata. Shouldn't we use the provided p id?
There was a problem hiding this comment.
If that is the default case we want, yes it has to be changed.
so the default would be overwrite the id in case the given one differs from the metadata id, what would we do with MetadataReadOnly in that case?
There was a problem hiding this comment.
In that case there's no write unless a save operation is explicitly called and I believe SaveMetadata was deleted.
jfontan
left a comment
There was a problem hiding this comment.
LGTM, just check the comment to see if it makes sense.
Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
782580c to
b4407c0
Compare
Closes #82