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

#25571: Improve the lltrace feature with the Py_Debug mode

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by stephane
Modified:
11 months, 3 weeks ago
Reviewers:
xavier.combelle, barry
CC:
haypo, akaptur, gwk, matrixise, xcombelle, ebarry
Visibility:
Public.

Patch Set 1 #

Total comments: 3

Patch Set 2 #

Patch Set 3 #

Total comments: 7

Patch Set 4 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/debug.rst View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
Doc/library/ltrace.rst View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
Lib/test/test_ltrace.py View 1 2 3 1 chunk +43 lines, -0 lines 0 comments Download
Makefile.pre.in View 1 2 3 2 chunks +10 lines, -1 line 0 comments Download
Python/ceval.c View 1 2 3 2 chunks +9 lines, -5 lines 0 comments Download
Python/makeopcodetranslations.py View 1 2 3 1 chunk +52 lines, -0 lines 0 comments Download
Python/opcode_translations.h View 1 2 3 1 chunk +258 lines, -0 lines 0 comments Download

Messages

Total messages: 2
xcombelle
http://bugs.python.org/review/25571/diff/15896/Python/makeopcodetranslations.py File Python/makeopcodetranslations.py (right): http://bugs.python.org/review/25571/diff/15896/Python/makeopcodetranslations.py#newcode9 Python/makeopcodetranslations.py:9: def find_module(modname): I feel like the name could be ...
1 year ago #1
ebarry
11 months, 3 weeks ago #2
Some comments, mostly doc-related nitpicks.

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst
File Doc/library/ltrace.rst (right):

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode1
Doc/library/ltrace.rst:1: ltrace --- ByteCode execution
This should be "Bytecode" (the C shouldn't be capitalized).

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode5
Doc/library/ltrace.rst:5: python interpreter.
Python the language should be written with a capital P.

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode10
Doc/library/ltrace.rst:10: In the Python interpreter, you have to define a
variable `__ltrace__` and set up
Use two backticks, e.g. ``__ltrace__``

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode30
Doc/library/ltrace.rst:30: >>>
Don't add a bare ">>> " at the end of examples.

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode34
Doc/library/ltrace.rst:34: the __ltrace__ variable from the globals dictionary::
"byte codes" should be "bytecode";
"in removing" should be "by deleting"; and
"__ltrace__" should be "``__ltrace__``".

About the "globals dictionary" (which should be "global namespace"), does that
also work if it's defined in the local namespace instead?

http://bugs.python.org/review/25571/diff/18048/Doc/library/ltrace.rst#newcode44
Doc/library/ltrace.rst:44: >>>
Same here, don't add ">>> " at the end.

http://bugs.python.org/review/25571/diff/18048/Lib/test/test_ltrace.py
File Lib/test/test_ltrace.py (right):

http://bugs.python.org/review/25571/diff/18048/Lib/test/test_ltrace.py#newcode4
Lib/test/test_ltrace.py:4: from test.support.script_helper import
(assert_python_ok, make_script)
No need for parentheses here (but it doesn't harm to have them either).
Sign in to reply to this message.

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