Skip to content
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

implement AddLocation in siva Library #10

Merged
merged 4 commits into from
Feb 21, 2019
Merged

implement AddLocation in siva Library #10

merged 4 commits into from
Feb 21, 2019

Conversation

jfontan
Copy link
Owner

@jfontan jfontan commented Feb 20, 2019

No description provided.

@codecov-io
Copy link

codecov-io commented Feb 20, 2019

Codecov Report

Merging #10 into siva will increase coverage by 2.83%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             siva      #10      +/-   ##
==========================================
+ Coverage   60.64%   63.48%   +2.83%     
==========================================
  Files          13       13              
  Lines         770      786      +16     
==========================================
+ Hits          467      499      +32     
+ Misses        232      210      -22     
- Partials       71       77       +6
Impacted Files Coverage Δ
util/storer.go 0% <ø> (ø) ⬆️
siva/library.go 68.47% <100%> (+2.59%) ⬆️
siva/location.go 57.14% <47.82%> (+4.15%) ⬆️
siva/checkpoint.go 63.38% <62.5%> (-1.7%) ⬇️
siva/repository.go 76% <75%> (+3.27%) ⬆️
plain/library.go 84.78% <0%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ed4072...42707ba. Read the comment docs.

}

// AddLocation creates a new borges.Location if it does not exist.
func (l *Library) AddLocation(id borges.LocationID) (borges.Location, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add locations should be used only to create new locations and left Location as the way to retrieve an already created location. So if AddLocation try to create an already existed siva file then it should return an error. wdyt?

siva/location.go Outdated
func NewLocation(id borges.LocationID, lib *Library, path string) (*Location, error) {
cp, err := newCheckpoint(lib.fs, path)
// OpenLocation opens a location and returns a Location struct.
func OpenLocation(id borges.LocationID, lib *Library, path string) (*Location, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new public method is used nowhere, should we keep it anyway?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably get rid of both CreateLocation and OpenLocation and use only NewLocation with the create parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even we could make NewLocation private because as library user you should use the Library to manage locations.

siva/location.go Outdated
}

// CreateLocation opens or creates a location and returns a Location struct.
func CreateLocation(id borges.LocationID, lib *Library, path string) (*Location, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new public method is used nowhere, should we keep it anyway?

@jfontan jfontan mentioned this pull request Feb 21, 2019
Signed-off-by: Javi Fontan <jfontan@gmail.com>
Also deleted unused Location functions

Signed-off-by: Javi Fontan <jfontan@gmail.com>
Signed-off-by: Javi Fontan <jfontan@gmail.com>
Signed-off-by: Javi Fontan <jfontan@gmail.com>
@jfontan jfontan merged commit e0f8f45 into siva Feb 21, 2019
jfontan added a commit that referenced this pull request Apr 8, 2019
siva: implement PackfileWriter and Filesystem in storer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants