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

packfile/scanner: reset zlib reader instead of new one #201

Merged
merged 2 commits into from
Jan 12, 2017
Merged

packfile/scanner: reset zlib reader instead of new one #201

merged 2 commits into from
Jan 12, 2017

Conversation

ajnavarro
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Jan 5, 2017

Current coverage is 76.09% (diff: 63.63%)

Merging #201 into master will decrease coverage by 0.62%

@@             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   

Powered by Codecov. Last update 133b97e...9e50460

@smola smola requested a review from mcuadros January 5, 2017 12:26
@@ -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) {
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Collaborator

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.

Copy link
Contributor

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)
Copy link
Contributor

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.

Copy link
Contributor Author

@ajnavarro ajnavarro Jan 9, 2017

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.

@mcuadros mcuadros merged commit a2ecbbc into src-d:master Jan 12, 2017
@ajnavarro ajnavarro deleted the feature/improve-scanner branch January 12, 2017 09:08
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.

5 participants