-
Notifications
You must be signed in to change notification settings - Fork 534
packfile/scanner: reset zlib reader instead of new one #201
Conversation
Current coverage is 76.09% (diff: 63.63%)@@ master #201 diff @@
==========================================
Files 96 96
Lines 6224 6232 +8
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 4775 4742 -33
- Misses 923 971 +48
+ Partials 526 519 -7
|
@@ -275,19 +276,28 @@ func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err erro | |||
// ReadRegularObject reads and write a non-deltified object | |||
// from it zlib stream in an object entry in the packfile. | |||
func (s *Scanner) copyObject(w io.Writer) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on this function starts with a wrong function name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is not related to this PR, Could fix this naming issue into another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please. Avoid unrelated changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, change it on another PR.
if s.zr == nil { | ||
zr, err := zlib.NewReader(s.r) | ||
if err != nil { | ||
return -1, fmt.Errorf("zlib initialization error: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we returning -1, this is not C!
Scanner.NextObject
first return argument is not used anywhere, it is safe to return 0 here instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just replicating the previous behavior. I can change it.
No description provided.