packfile: delta selection logic #182
Conversation
ajnavarro
commented
Dec 13, 2016
ajnavarro
commented
- Implemented logic to assign deltas to objects
|
Please, merge #172 first |
| . "gopkg.in/check.v1" | ||
| ) | ||
|
|
||
| type DeltaSelectorSuite struct { |
There was a problem hiding this comment.
@smola @mcuadros @alcortesm Some ideas for testing deltaSelector more deeply?
| ) | ||
|
|
||
| const ( | ||
| // deltas based on deltas, how much steps we can do |
There was a problem hiding this comment.
"how much" -> "how many"
clarify that it is 50 as implemented in jgit
|
|
||
| type deltaSelector struct { | ||
| storer storer.ObjectStorer | ||
| depths map[plumbing.Hash]int |
| ) | ||
|
|
||
| // set of object types that we should apply deltas | ||
| var applyDelta = map[plumbing.ObjectType]bool{ |
There was a problem hiding this comment.
"set of object types that we should apply deltas" -> "applyDelta is the set of object types that we should apply deltas"
| } | ||
|
|
||
| func newDeltaSelector(s storer.ObjectStorer) *deltaSelector { | ||
| return &deltaSelector{ |
| } | ||
| } | ||
|
|
||
| func (dw *deltaSelector) getObjectsToPack(hashes []plumbing.Hash) ([]*plumbing.ObjectToPack, error) { |
There was a problem hiding this comment.
getObjectsToPack -> objectsToPack
|
|
||
| // GetObjectsToPack creates a list of ObjectToPack from the hashes provided, | ||
| // creating deltas if it's suitable, using an specific internal logic | ||
| func (dw *deltaSelector) GetObjectsToPack(hashes []plumbing.Hash) ([]*plumbing.ObjectToPack, error) { |
There was a problem hiding this comment.
GetObjectsToPack -> ObjectsToPack
There was a problem hiding this comment.
Put public methods first in the file.
| } | ||
| } | ||
|
|
||
| return deltaToPack, nil |
There was a problem hiding this comment.
instead of returning a new ObjectToPack, you could just set the base, delta and depth. Maybe something like:
target.SetBase(*ObjectToPack)
| s.ds = newDeltaSelector(s.store) | ||
| } | ||
|
|
||
| func (s *DeltaSelectorSuite) TestTryOrder(c *C) { |
| c.Assert(toSort, DeepEquals, expected) | ||
| } | ||
|
|
||
| func (s *DeltaSelectorSuite) TestTryToDeltify(c *C) { |
There was a problem hiding this comment.
Test ObjectstoPack method, instead of the internal tryToDeltify?
| // Delta Size Limit with no best delta yet | ||
| bestDelta, err = s.ds.tryToDeltify(base, target) | ||
| c.Assert(err, IsNil) | ||
| //c.Assert(bestDelta, IsNil) |
Current coverage is 77.51% (diff: 87.37%)@@ master #182 diff @@
==========================================
Files 93 94 +1
Lines 5953 6017 +64
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 4631 4664 +33
- Misses 836 879 +43
+ Partials 486 474 -12
|
|
@ajnavarro rebase |
|
@mcuadros rebased |
| @@ -0,0 +1,28 @@ | |||
| package packfile | |||
|
|
|||
| import "gopkg.in/src-d/go-git.v4/plumbing" | |||
There was a problem hiding this comment.
if you put a common_test.go, put the "testing" hook here.
| break | ||
| } | ||
|
|
||
| err := dw.tryToDeltify(base, target) |
| return err | ||
| } | ||
|
|
||
| // if delta better than target |
There was a problem hiding this comment.
I don't think this should go inside an if block.
- Implemented logic to assign deltas to objects
* packfile: delta selection logic - Implemented logic to assign deltas to objects * Requested changes * Improved tests and fix errors
Disable HTTP caching when the upload is OneShot
