docs: clarify pytestmark phase and soften import style rule#4616
Open
EdDev wants to merge 2 commits intoRedHatQE:mainfrom
Open
docs: clarify pytestmark phase and soften import style rule#4616EdDev wants to merge 2 commits intoRedHatQE:mainfrom
EdDev wants to merge 2 commits intoRedHatQE:mainfrom
Conversation
During Phase 1 (STD), markers are listed in the Markers: docstring section. During Phase 2 (implementation), they are converted to pytestmark expressions or @pytest.mark decorators. This was not documented, causing confusion when reviewing implementation PRs that add pytestmark where the STD placeholder had none. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Edward Haas <edwardh@redhat.com>
The previous blanket rule ("ALWAYS import specific functions, NEVER
import module") conflicted with cases where retaining the module name
at the call site genuinely improves readability (e.g.
libstuntime.ContinuousPing makes the origin clear, whereas a bare
ContinuousPing does not). Soften to a context-sensitive guideline:
prefer specific imports for functions and constants, allow module
imports when namespace context adds clarity.
Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Edward Haas <edwardh@redhat.com>
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-2
requested review from
RoniKishner and
vsibirsk
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Two documentation clarifications surfaced during PR review:
pytestmarkis a Phase 2 addition — during STD placeholders (Phase 1), markers live in theMarkers:docstring section. During implementation (Phase 2), they are converted topytestmarkexpressions or@pytest.markdecorators. This was undocumented, causing reviewers to flag the absence ofpytestmarkin STDs or its presence in implementation PRs as incorrect.Import style rule was too strict — the previous rule ("ALWAYS import specific functions, NEVER
import module") conflicted with cases where retaining the module name at the call site genuinely improves readability (e.g.libstuntime.ContinuousPingmakes the origin clear; a bareContinuousPingdoes not). The rule is softened to a context-sensitive guideline.Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket: