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

#14328: Add keyword-only parameter support to PyArg_ParseTupleAndKeywords

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by larry
Modified:
7 years ago
Reviewers:
benjamin, georg
CC:
gregory.p.smith, AntoinePitrou, larry, Benjamin Peterson, Arfrever, devnull_psf.upfronthosting.co.za, eric.snow, devnull_psf.upfronthosting.co.za
Visibility:
Public.

Patch Set 1 #

Total comments: 6

Patch Set 2 #

Patch Set 3 #

Total comments: 12

Patch Set 4 #

Total comments: 6

Patch Set 5 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/c-api/arg.rst View 1 chunk +9 lines, -0 lines 0 comments Download
Lib/test/test_getargs2.py View 1 2 3 3 chunks +73 lines, -1 line 0 comments Download
Modules/_testcapimodule.c View 1 2 3 4 3 chunks +19 lines, -1 line 0 comments Download
Python/getargs.c View 1 2 3 4 3 chunks +33 lines, -1 line 0 comments Download

Messages

Total messages: 6
Benjamin Peterson
http://bugs.python.org/review/14328/diff/4428/15843 File Lib/test/test_getargs2.py (right): http://bugs.python.org/review/14328/diff/4428/15843#newcode348 Lib/test/test_getargs2.py:348: try: Use assertRaises. http://bugs.python.org/review/14328/diff/4428/15843#newcode351 Lib/test/test_getargs2.py:351: self.assertEqual(str(err), "Function takes at ...
7 years ago #1
larry
http://bugs.python.org/review/14328/diff/4428/15843 File Lib/test/test_getargs2.py (right): http://bugs.python.org/review/14328/diff/4428/15843#newcode348 Lib/test/test_getargs2.py:348: try: On 2012/03/16 02:21:47, Benjamin Peterson wrote: > Use ...
7 years ago #2
Benjamin Peterson
http://bugs.python.org/review/14328/diff/4444/15874 File Doc/c-api/arg.rst (right): http://bugs.python.org/review/14328/diff/4444/15874#newcode342 Doc/c-api/arg.rst:342: Indicates that the remaining arguments in the Python argument ...
7 years ago #3
larry
Thank you for the lovely, thorough feedback Benjamin! I incorporated all your changes that didn't ...
7 years ago #4
Georg
http://bugs.python.org/review/14328/diff/4454/15896 File Doc/c-api/arg.rst (right): http://bugs.python.org/review/14328/diff/4454/15896#newcode345 Doc/c-api/arg.rst:345: arguments, so ``|`` must always be specified before ``$`` ...
7 years ago #5
larry
7 years ago #6
Done.  I saw this then promptly forgot about it!  (Benjamin reminded me.)

http://bugs.python.org/review/14328/diff/4454/15896
File Doc/c-api/arg.rst (right):

http://bugs.python.org/review/14328/diff/4454/15896#newcode345
Doc/c-api/arg.rst:345: arguments, so ``|`` must always be specified before ``$``
in the format string.
On 2012/03/17 19:15:19, Georg wrote:
> Needs a versionadded/versionchanged.
> 
> Also, looks like the line is too long.

Done.

http://bugs.python.org/review/14328/diff/4454/15898
File Modules/_testcapimodule.c (right):

http://bugs.python.org/review/14328/diff/4454/15898#newcode830
Modules/_testcapimodule.c:830: &required, &optional, &keyword_only))
On 2012/03/17 19:15:19, Georg wrote:
> Nitpickingly: PEP 7 says this should be indented under "args".

*sob* Done.  I think that's my least favorite thing in PEP 7.

http://bugs.python.org/review/14328/diff/4454/15899
File Python/getargs.c (right):

http://bugs.python.org/review/14328/diff/4454/15899#newcode1458
Python/getargs.c:1458: "Invalid format string (| specified twice)");
On 2012/03/17 19:15:19, Georg wrote:
> Indentation seems to be off here and for the next two PyErr_SetStrings.

Done.
Sign in to reply to this message.

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