| msg172655 - (view) |
Author: Zachary Ware (zach.ware) *  |
Date: 2012-10-11 16:12 |
| There are several small inconsistencies in the winreg module documentation (docstrings and winreg.rst). Mostly these are discrepancies between "res" and "reserved", "sam" and "access", and whether or not those two are keyword arguments, but there are some other missing pieces as well. I'm happy to provide a patch to correct everything, but I'd like a little guidance beforehand. I have no experience with docstrings in C code, so I've attached a patch that attempts to correct just the CreateKeyEx() docstring. If it turns out to be ok, I'll go ahead and create a patch fixing the other ones. I'm comfortable enough with ReST changes that I'll just include those with the final docstring patch. Also, some of the changes are specific to 3.3+ (due to the WindowsError => OSError change). Should I provide one patch against default, two against 3.2 and 3.3 (including the 3.2 changes), or two against 3.2 and 3.3 (after a local commit of the 3.2 changes)? Any of the above is perfectly doable :) Thanks, Zach |
|
|
| msg172657 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2012-10-11 16:21 |
| The patch looks alright, but I would remove the lining up of definitions. It's probably easiest to do a patch against 3.2 and I can handle the porting on commit. |
|
|
| msg172661 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-11 16:38 |
| Okie doke. I'll try to have a patch ready for review later this afternoon. |
|
|
| msg172672 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-11 18:41 |
| Here's the patch against 3.2, hopefully I caught everything I meant to :) Unfortunately, I can't build Python or the docs on this machine, so I can't guarantee I didn't break anything. I tried to be careful though, especially with changing linebreaks in the docstrings. The changes for 3.3+ that don't apply to 3.2 are: - in PC\winreg.c - Change all occurrences of "a WindowsError" to "an OSError" - in Doc\library\winreg.rst - Change "a" to "an" in all occurrences of "a :exc:`OSError`" - Consolidate all the ..versionchanged:: 3.3 notes into one at the top (like in Doc\library\msvcrt.rst) Also, I did make a couple of non-cosmetic informational changes other than on 'res' and 'sam', such as listing the correct default 'access' parameter on a couple of functions (judging by my reading of said functions). Thanks, Zach |
|
|
| msg172994 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-15 18:34 |
| It occurs to me that I should have asked; should the documentation be changed to match the code, or the code to match the documentation (with regards to the default argument to the access parameter in a few functions)? |
|
|
| msg172995 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2012-10-15 18:39 |
| Docs should match code. If we did it the other way around we'd probably break something. Thanks for looking into this. I've been busy the last few days but I will get to the review and application of the patch very soon. |
|
|
| msg172997 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-15 19:20 |
| That's what I figured, so that's what I did in the patch; but I've also seen cases in Python where prior documentation has dictated how the code should work. Thanks for the confirmation. |
|
|
| msg174162 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-29 23:21 |
| New changeset b9e14b89199b by Brian Curtin in branch '3.2': Fix #16197. Update docstrings and documentation to match winreg code. http://hg.python.org/cpython/rev/b9e14b89199b |
|
|
| msg174163 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2012-10-29 23:22 |
| Pushed fixes for 3.2+ Thanks for the patch! |
|
|
| msg174218 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-30 19:24 |
| Thanks for the commit! 3.2 looks good now. 3.3 and default still need a little work, though; the docstrings still say "WindowsError" instead of "OSError" and the docs say "a OSError" instead of "an OSError". The attached patch cleans up both issues, as well as consolidating all the versionchanged notices about WindowsError == OSError. |
|
|
| msg174271 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-31 11:15 |
| Not sure consolidating is good idea, ok with other changes. |
|
|
| msg174279 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-31 14:12 |
| The thought on consolidating is to match Doc/library/msvcrt.rst which does the same thing. Also, when I started reading this page shortly before opening this issue, I was reading most of the page at once and was frankly pretty annoyed by seeing the same notice over and over. Although I could see the point of having a much shorter blurb on each affected function in addition to an explanatory note at the top. So instead of ''' .. versionchanged:: 3.3 This function used to raise a :exc:`WindowsError`, which is now an alias of :exc:`OSError`. ''' on each one, something like ''' .. versionchanged:: 3.3 :exc:`WindowsError` is :exc:`OSError` ''' perhaps? |
|
|
| msg174289 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-31 15:10 |
| What's about compromise from attached file? |
|
|
| msg174291 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-31 15:25 |
| Not bad, but with that scheme we could even go as far as cutting out the 'WindowsError is OSError' bit and make it just """ ..versionchanged:: 3.3 See :ref:`above `. """ |
|
|
| msg174322 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-31 16:37 |
| I have updated patch. |
|
|
| msg174335 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2012-10-31 17:15 |
| v3 looks fine to me! |
|
|
| msg174337 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-31 17:30 |
| New changeset 2e7da832219d by Andrew Svetlov in branch '3.3': Issue #16197: Fix several small errors in winreg documentation. http://hg.python.org/cpython/rev/2e7da832219d New changeset f1310219c702 by Andrew Svetlov in branch 'default': Merge issue #16197: Fix several small errors in winreg documentation. http://hg.python.org/cpython/rev/f1310219c702 |
|
|
| msg174338 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-31 17:31 |
| Committed. Thanks, Zachary! |
|
|