Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1171)

Issue 63060: [issue5804] Add an 'offset' argument to zlib.decompress

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 10 months ago by gregory.p.smith
Modified:
14 years, 10 months ago
Reviewers:
CC:
kristjan_ccpgames.com, report_bugs.python.org
Base URL:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Description

From http://bugs.python.org/issue5804 starting with zlibpatch2.patch.

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -9 lines) Patch
Doc/library/zlib.rst View 2 chunks +10 lines, -1 line 2 comments Download
Lib/test/test_zlib.py View 2 chunks +21 lines, -1 line 1 comment Download
Modules/zlibmodule.c View 3 chunks +24 lines, -7 lines 2 comments Download

Messages

Total messages: 2
gregory.p.smith
14 years, 10 months ago (2009-05-10 20:53:21 UTC) #1
gregory.p.smith
14 years, 10 months ago (2009-05-10 21:05:27 UTC) #2
Overall, this looks good.  Some mostly minor comments in this review.

http://codereview.appspot.com/63060/diff/1/2
File Doc/library/zlib.rst (right):

http://codereview.appspot.com/63060/diff/1/2#newcode136
Line 136: When specified, it will cause the function's return value to be a
(uncompressed, offset)
how about this wording:

..."to be a tuple of (uncompressed, offset), with the"...

http://codereview.appspot.com/63060/diff/1/2#newcode142
Line 142: Added the *offset* argument
missing . at the end of the sentence.  Also, mention that this function now
accepts keyword arguments.

http://codereview.appspot.com/63060/diff/1/3
File Lib/test/test_zlib.py (right):

http://codereview.appspot.com/63060/diff/1/3#newcode110
Line 110: c = zlib.compress(a)+zlib.compress(b)
please surround the + with spaces for readability and consistency with other
code in the file.\

http://codereview.appspot.com/63060/diff/1/4
File Modules/zlibmodule.c (right):

http://codereview.appspot.com/63060/diff/1/4#newcode193
Line 193: "the start position in the string to start decompresion and, if
spceified,\n"
typo: specified.

http://codereview.appspot.com/63060/diff/1/4#newcode296
Line 296: offset = length-zst.avail_in+offset;
leave spaces around your - and + operators and this becomes easier to read.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b