NO-JIRA: add --max-depth flag to nav order verification#8372
NO-JIRA: add --max-depth flag to nav order verification#8372openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
jparrill
changed the title
📝 WalkthroughWalkthroughThe script hack/verify-docs-nav-order.py now accepts a --max-depth command-line argument to control recursive validation depth of nested navigation sections. The check_section_order function signature was updated to include a max_depth parameter (default 1). The function only recurses into subsections when the depth limit permits, decrementing the counter at each nesting level; -1 denotes unlimited recursion. Alphabetical ordering checks and error reporting behavior for ordering violations are unchanged. 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 46 seconds. Comment |
openshift-ci-robot
added
the
jira/valid-reference
|
@jparrill: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira rerfesh |
|
/area ci-tooling |
openshift-ci
Bot
added
area/ci-tooling
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/verify-docs-nav-order.py`:
- Around line 140-145: The --max-depth flag currently uses type=int which allows
0 and other negatives that the recursion treats as unlimited; update the
argument handling for parser.add_argument('--max-depth', ...) so that after
parsing you validate args.max_depth is either -1 or a positive integer (>0) and
call parser.error(...) (or raise argparse.ArgumentTypeError via a custom type)
for any other value (e.g., 0 or < -1) to enforce the documented semantics;
reference the parser, the add_argument call for '--max-depth' and args.max_depth
when locating and applying this validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1a79ac0d-8efe-4307-9f8e-7ea46d412beb
📒 Files selected for processing (1)
hack/verify-docs-nav-order.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8372 +/- ##
=======================================
Coverage 36.46% 36.46%
=======================================
Files 765 765
Lines 93256 93256
=======================================
Hits 34010 34010
Misses 56532 56532
Partials 2714 2714
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The script previously checked alphabetical order at all nav levels recursively. Add a --max-depth CLI argument to control how many levels deep to verify (default: 1 = top-level only, -1 = unlimited). Reject invalid values like 0 or < -1 that have undefined behavior. Also reorder Disaster Recovery nav to place Prerequisites first. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hack/verify-docs-nav-order.py (1)
130-130: Use iterator access instead of materializing a list.Line 130 can avoid creating an intermediate list for a single value lookup.
♻️ Proposed micro-refactor
- value = list(entry.values())[0] + value = next(iter(entry.values()))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/verify-docs-nav-order.py` at line 130, Replace the materialization of a list for a single lookup by using an iterator: instead of value = list(entry.values())[0], obtain the first value with next(iter(entry.values())) (or next(iter(entry.values()), None) if empty dicts must be handled) so you avoid creating an intermediate list; update the assignment where variable entry is used to use this iterator-based access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hack/verify-docs-nav-order.py`:
- Line 130: Replace the materialization of a list for a single lookup by using
an iterator: instead of value = list(entry.values())[0], obtain the first value
with next(iter(entry.values())) (or next(iter(entry.values()), None) if empty
dicts must be handled) so you avoid creating an intermediate list; update the
assignment where variable entry is used to use this iterator-based access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7186bf5d-e7ba-44f0-a4e6-79de76af30b9
📒 Files selected for processing (1)
hack/verify-docs-nav-order.py
openshift-ci
Bot
added
the
area/documentation
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hack/verify-docs-nav-order.py (1)
129-131: Extract single dict value more idiomatically.On line 130,
list(entry.values())[0]unnecessarily allocates an intermediate list. Usenext(iter(entry.values()))instead. Note: the same pattern appears on line 69 and should also be refactored.♻️ Proposed refactor
- value = list(entry.values())[0] + value = next(iter(entry.values()))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/verify-docs-nav-order.py` around lines 129 - 131, Replace the non-idiomatic extraction of the sole dict value that uses list(entry.values())[0] with an iterator-based approach to avoid allocating an intermediate list; update both occurrences (the one shown around the conditional that checks isinstance(entry, dict) and the similar pattern earlier around line 69) to use next(iter(entry.values())) when pulling the single value from entry so the variable handling (entry -> value) remains identical but without the extra list allocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hack/verify-docs-nav-order.py`:
- Around line 129-131: Replace the non-idiomatic extraction of the sole dict
value that uses list(entry.values())[0] with an iterator-based approach to avoid
allocating an intermediate list; update both occurrences (the one shown around
the conditional that checks isinstance(entry, dict) and the similar pattern
earlier around line 69) to use next(iter(entry.values())) when pulling the
single value from entry so the variable handling (entry -> value) remains
identical but without the extra list allocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4cc6100c-337a-4330-838e-79c2a9569d6c
📒 Files selected for processing (2)
docs/mkdocs.ymlhack/verify-docs-nav-order.py
✅ Files skipped from review due to trivial changes (1)
- docs/mkdocs.yml
|
/verified by presubmits |
openshift-ci-robot
added
the
verified
|
@jparrill: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
Test Resultse2e-aws
e2e-aks
|
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest |
|
I now have all the evidence needed. Here is the complete analysis: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe Root CauseAzure vCPU Quota Exhaustion (Infrastructure Issue — NOT related to PR) The failure chain:
PR #8372 only modifies Recommendations
Evidence
|
|
@jparrill: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
--max-depthCLI argument tohack/verify-docs-nav-order.pyto control how many nav levels are checked for alphabetical order1(top-level only), use-1for unlimited (previous behavior)Test plan
python3 hack/verify-docs-nav-order.pypasses with default (top-level only)python3 hack/verify-docs-nav-order.py --max-depth -1checks all levelspython3 hack/verify-docs-nav-order.py --max-depth 2checks two levels deep🤖 Generated with Claude Code
Summary by CodeRabbit