[docs] Add that type hinting is unnecessary for test cases#3496
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the developer coding guidelines documentation to clarify that type hints are not required for test methods, aligning expectations for writing tests in this codebase.
Changes:
- Adds a note stating type hinting is unnecessary for test functions/methods.
- Extends the “Docstrings” guidance for test methods.
| In test classes, it is recommended to have one-line docstring for each test method. For very short and obvious | ||
| tests, where the function name explain what all it does, then in such cases, the docstring can be omitted. | ||
| tests, where the function name explain what all it does, then in such cases, the docstring can be omitted. As test | ||
| functions only have self as input argument and no return value, type hinting in the function signature is not required. | ||
|
|
||
| For e.g. 2.3 the test class should be defined as |
📝 WalkthroughWalkthroughThe coding guidelines documentation ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
doc/develop/coding-guidelines.rst (1)
163-164: 💤 Low valueImprove grammar in line 163.
The phrase "explain what all it does" is slightly awkward; it should read "explain what it does" for standard English.
✏️ Proposed fix
- tests, where the function name explain what all it does, then in such cases, the docstring can be omitted. As test + tests, where the function name explains what it does, then in such cases, the docstring can be omitted. As test🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@doc/develop/coding-guidelines.rst` around lines 163 - 164, In the documentation about test function naming conventions, correct the grammatical error in the phrase that currently reads "explain what all it does". Change this phrase to "explain what it does" to follow standard English grammar. This appears in the context of describing when docstrings can be omitted for test functions with self as the only input argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@doc/develop/coding-guidelines.rst`:
- Around line 163-164: In the documentation about test function naming
conventions, correct the grammatical error in the phrase that currently reads
"explain what all it does". Change this phrase to "explain what it does" to
follow standard English grammar. This appears in the context of describing when
docstrings can be omitted for test functions with self as the only input
argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9d7f04c-ef49-4130-8262-9f3fa3707e4d
📒 Files selected for processing (1)
doc/develop/coding-guidelines.rst
No description provided.