bpo-38235: Correct some arguments names in logging documentation #16571
Conversation
There was a problem hiding this comment.
Thanks for the PR @AWhetter and welcome!
I'm not sure if the preference is for consistency or readability.
Typically, that decision is left up to the maintainer for the module, rather than a global rule for all of the documentation. In the case of logging, that is @vsajip. I'll add him as a reviewer.
Personally, I'm in favor of changing lvl to level for consistency. I'm generally not a fan of abbreviating argument names.
I'm happy to find and fix the other inconsistencies if that's preferred.
As for fixing all of the other inconsistencies, let's keep this PR simple and focused just on the lvl to level conversions. That makes it much easier to review and makes merging the PR a much smaller decision for the maintainer to make. In it's current state, I think it works great as an introductory PR to gain some experience with the CPython workflow. (:
|
Also @vsajip would you mind being added to the CODEOWNERS file as an owner for anything related to the logging module? This would allow you to be automatically added as a reviewer for any PR that affects logging documentation or code files. I don't mind opening a PR to add you if you don't have the time to do so, but I wanted to make sure you were okay with it first. Brett was saying that he wants to encourage populating CODEOWNERS with experts instead of manually adding reviewers. |
|
GH-16576 is a backport of this pull request to the 3.8 branch. |
|
GH-16577 is a backport of this pull request to the 3.7 branch. |
…honGH-16571) (cherry picked from commit 3142c66) Co-authored-by: Ashley Whetter <AWhetter@users.noreply.github.com>
|
Congrats on your first CPython contribution @AWhetter! 🍾 Looking forward to seeing more from you in the future. |
|
Yay! Thank you! 🙌 |
pythonGH-16571). (cherry picked from commit 3142c66) Co-authored-by: Ashley Whetter <AWhetter@users.noreply.github.com>
…honGH-16571) (pythonGH-16577) (cherry picked from commit 3142c66) Co-authored-by: Ashley Whetter <AWhetter@users.noreply.github.com>
There's some other arguments names that are also incorrect in the documentation. For example
Formatter.formatExceptionis defined asdef formatException(self, ei):but documented as.. method:: formatException(exc_info), andmakeLogRecordis defined asdef makeLogRecord(dict):but documented as.. function:: makeLogRecord(attrdict). The argument names in the documentation are more clear, but they don't match the code. I'm not sure if the preference is for consistency or readability. So for now I've only changedlvlbecauselevelis a better name, but I'm happy to find and fix the other inconsistencies if that's preferred.https://bugs.python.org/issue38235