RepositoryIterator and ReferenceIterator implementations #20
Conversation
Merge #19 first. |
Codecov Report
@@ Coverage Diff @@
## master #20 +/- ##
============================================
- Coverage 88.46% 86.63% -1.83%
- Complexity 11 25 +14
============================================
Files 6 10 +4
Lines 182 232 +50
Branches 17 23 +6
============================================
+ Hits 161 201 +40
- Misses 14 19 +5
- Partials 7 12 +5
Continue to review full report at Codecov.
|
| } catch { | ||
| case _: URISyntaxException => None | ||
| } | ||
| }).distinct.min |
There was a problem hiding this comment.
Is min used because of something specific or just to get one of the results?
There was a problem hiding this comment.
min === sorted.head
There was a problem hiding this comment.
yes, that's what I meant, do we need them sorted or do we just want the first?
There was a problem hiding this comment.
we want the first after sort them.
There was a problem hiding this comment.
Discussed IRL: sorted is needed, lgtm then
There was a problem hiding this comment.
I must admit that it's not clear to me either, why sorting is needed.
Could be a good idea to document that
| } | ||
|
|
||
| object RepositoryProvider { | ||
| var provider: RepositoryProvider = _ |
There was a problem hiding this comment.
if this is a singleton, what about moving everything to the RepositoryProvider object instead of having the class and manually manage the singleton?
Also, if you do RepositoryProvider("foo") and then RepositoryProvider("bar") what you get is a repository provider with "foo" as localPath, which is a bit misleading.
There was a problem hiding this comment.
This code is from another PR. Can you comment this there?: #19
There was a problem hiding this comment.
| object SivaRDDProvider { | ||
| var provider: SivaRDDProvider = _ | ||
|
|
||
| def apply(sc: SparkContext): SivaRDDProvider = { |
There was a problem hiding this comment.
same as with RepositoryProvider
There was a problem hiding this comment.
This code is from another PR. Can you comment this there?: #19
There was a problem hiding this comment.
Yes, sorry about that, but at this early stage on the project, is really difficult split functionality and go forward without depending of another in-process functionalities. |
Using a base abstract class RootedRepoIterator, we add two implementations, one of them to iterate repository metadata (repository id, urls, is fork) and references metadata (repository_id, name, hash). With this RootedRepoIterator we should be able to implement CommitIterator and BlobIterator too. Filter logic must be implemented before start with BlobIterator. - Split test logic into Traits to be able to use them in all the Specs. - Added a BaseRootedRepoIterator trait with a helper to test iterators more easly.
ajnavarro
force-pushed
the
feature/repository-iterator
branch
from
402afa9
to
53ced47
Compare
Using a base abstract class RootedRepoIterator, we add two implementations, one of them to iterate repository metadata (repository id, urls, is fork) and references metadata (repository_id, name, hash).
With this RootedRepoIterator we should be able to implement CommitIterator and BlobIterator too.
Filter logic must be implemented before start with BlobIterator.