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

#25638: Verify the etree_parse and etree_iterparse benchmarks are working appropriately

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by brett
Modified:
2 years ago
Reviewers:
storchaka
CC:
brett.cannon, AntoinePitrou, scoder, eli.bendersky, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 11

Patch Set 3 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Modules/_elementtree.c View 1 2 3 chunks +19 lines, -7 lines 2 comments Download

Messages

Total messages: 4
brett.cannon
Basically LGTM, just had some questions. http://bugs.python.org/review/25638/diff/16051/Lib/xml/etree/ElementTree.py File Lib/xml/etree/ElementTree.py (right): http://bugs.python.org/review/25638/diff/16051/Lib/xml/etree/ElementTree.py#newcode1210 Lib/xml/etree/ElementTree.py:1210: data = source.read(16 ...
2 years ago #1
storchaka_gmail.com
http://bugs.python.org/review/25638/diff/16051/Lib/xml/etree/ElementTree.py File Lib/xml/etree/ElementTree.py (right): http://bugs.python.org/review/25638/diff/16051/Lib/xml/etree/ElementTree.py#newcode1210 Lib/xml/etree/ElementTree.py:1210: data = source.read(16 * 1024) On 2015/12/05 00:30:00, brett.cannon ...
2 years ago #2
brett.cannon
LGTM http://bugs.python.org/review/25638/diff/16136/Modules/_elementtree.c File Modules/_elementtree.c (right): http://bugs.python.org/review/25638/diff/16136/Modules/_elementtree.c#newcode2502 Modules/_elementtree.c:2502: else I would avoid the bare `else` clause ...
2 years ago #3
storchaka_gmail.com
2 years ago #4
http://bugs.python.org/review/25638/diff/16136/Modules/_elementtree.c
File Modules/_elementtree.c (right):

http://bugs.python.org/review/25638/diff/16136/Modules/_elementtree.c#newcode...
Modules/_elementtree.c:2502: else
On 2015/12/09 22:04:23, brett.cannon wrote:
> I would avoid the bare `else` clause and still putting in the curly braces.

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7