Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Conversation

@JoshuaSjoding
Copy link
Contributor

The current implementation of the Repository struct includes a pointer to core.RAWObjectStorage. This seems inappropriate because it prevents the development of alternative implementations of ObjectStorage. I presume that the direct reference was included because it provided convenient and fast access to the commit objects.

This pull request proposes the addition of one function to the ObjectStorage interface: IterType. This function returns an iterator of objects matching the given object type. The Repository is modified to make use of this new function, and RAWObjectStorage is modified to provide it.

The return value of IterType is a new interface called ObjectIter. Two implementations of ObjectIter are provided:

  • ObjectLookupIter, which iterates over a slice of object hashes and looks up the given objects as needed. This is used to iterate through parent commits, for example.
  • ObjectSliceIter, which iterates over a slice of objects.

Additional implementations could easily be added.

Both CommitIter and TreeIter are modified to make use of the new ObjectIter interface, as is Commit.Parents(). Note that this does away with the previous channel-based iterators.

The proposed implementation of RAWObjectStorage.IterType simply flattens the map of the requested type into a slice on the fly. This is probably fine for small to medium repositories, but might cause trouble for large repositories or if it's called frequently. Further optimization should be done as warranted.

Note that this pull request modifies several public types. As such it should probably be considered breaking for v2; it may be more appropriate to pull this instead into a v3 branch. I expect that other breaking changes will be needed to meet the needs of #19, and I suspect that all such work should be carried out in a branch until it's ready to be rolled out formally as v3. Feedback on this is most welcome.

core/object.go Outdated
New() Object
Set(Object) Hash
Get(Hash) (Object, bool)
IterType(ObjectType) ObjectIter
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer call this method just Iter, is less verbose and clear enough

@mcuadros
Copy link
Contributor

@JoshuaSjoding great job, we will merge this as soon as the minor things I commented get fixed. You can keep the PR to master, because we are using gopkg.in, is not a problem having an unreleased master.

Thanks!

@JoshuaSjoding
Copy link
Contributor Author

@mcuadros I think that I've addressed all of your comments with the latest commit.

CommitIter has some decent test coverage now. Tree probably needs some work and should use a FileIter to be consistent, but I'll handle that with a separate issue and PR if it's wanted.

I also merged in the latest work from src-d:master for convenience. Let me know if you'd prefer that I not do that in the future.

@codecov-io
Copy link

Current coverage is 58.51%

Merging #20 into master will decrease coverage by -2.97% as of 99f8359

@@            master     #20   diff @@
======================================
  Files           24      24       
  Stmts         1532    1567    +35
  Branches       203     200     -3
  Methods          0       0       
======================================
- Hit            942     917    -25
- Partial         92      93     +1
- Missed         498     557    +59

Review entire Coverage Diff as of 99f8359

Powered by Codecov. Updated on successful CI builds.

@JoshuaSjoding
Copy link
Contributor Author

@mcuadros I went ahead and updated the ObjectStorage functions New, Get and Set so that they can return errors now.

@mcuadros mcuadros force-pushed the master branch 2 times, most recently from 72de676 to ef6652d Compare February 16, 2016 16:37
mcuadros added a commit that referenced this pull request Feb 16, 2016
Iterable ObjectStorage interface for use in Repository struct
@mcuadros mcuadros merged commit ccc7a2c into src-d:master Feb 16, 2016
@JoshuaSjoding JoshuaSjoding deleted the generic-object-storage branch June 24, 2016 09:08
mcuadros added a commit that referenced this pull request Jan 31, 2017
Iterable ObjectStorage interface for use in Repository struct
gsalingu-ovhus pushed a commit to gsalingu-ovhus/go-git that referenced this pull request Mar 28, 2019
Remove unused params in config
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants