fix(generator): preserve gcloud resource arguments before regeneration - #5113
Conversation
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Greptile SummaryThis PR preserves gcloud resource operands and selectors when regenerating typed command options.
Confidence Score: 5/5The PR appears safe to merge with no outstanding actionable findings. The current implementation consistently merges declared gcloud resource operands with synopsis metadata, validates that grouped arguments are emitted, and the post-review helper extraction does not change parser behavior.
|
| Filename | Overview |
|---|---|
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs | Integrates shared synopsis parsing with nested gcloud resource declarations and validates preserved operands and selectors. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/UsageSynopsisParser.cs | Adds colon-group and wrapped-synopsis handling while preserving explicitly nested alternatives; the latest helper extraction is behavior-neutral. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliScraperBase.cs | Extends grouped-argument coverage validation to positional operands. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/GcloudResourceArgumentTests.cs | Covers resource operands, selectors, ordering, repetition, requiredness, generation, and coverage failures. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[gcloud help text] --> B[Extract SYNOPSIS]
A --> C[Parse FLAGS and POSITIONAL ARGUMENTS]
B --> D[Parse operand order and requiredness]
C --> E[Build argument groups]
E --> F[Separate positional operands and named selectors]
D --> G[Merge synopsis metadata with declarations]
F --> G
G --> H[Validate every grouped argument is emitted]
H --> I[Generate typed options and services]
Reviews (2): Last reviewed commit: "refactor(generator): separate optional n..." | Re-trigger Greptile
|
Addressed CodeFactor's new complexity finding in 8ea531d. The report flags TryParseNestedOperandGroup (complexity 16). Its optional nested-operand loop now lives in TryParseOptionalNestedOperands, separating wrapper/colon validation from token-to-operand conversion without changing parsing rules, positions, option associations, requiredness, or diagnostics. All 894 scraper tests pass, including nested option association, option terminators, colon groups, and captured gcloud resource fixtures. Release generator solution build has zero warnings/errors; scoped severity-info formatting/verification and git diff --check pass. Existing tests cover this behavior-preserving refactor; no test or analyzer was disabled. No generated integration files changed. CodeFactor and the subsequent CI/review cycle must confirm the new head before merge. Parent #4850 remains open for current-main gcloud regeneration and integration validation. |
1 participant
Nested gcloud resource operands and selectors disappear from generated typed options because the scraper only recognizes flat positional declarations. This source prerequisite extracts the complete correction from #4956 so source can land before fresh latest-main generation.
Resource groups share the argument-group parser and shared SYNOPSIS result. Operand order, repetition, requiredness, configured selectors, fully qualified names, synopsis metadata, required alternatives, and coverage validation remain intact. Wrapped synopsis lines join their command, while inconsistent declarations and ambiguous top-level colon alternatives fail explicitly. Explicitly nested alternatives remain supported, and GCLOUD_WIDE_FLAG is excluded from operand metadata.
Real KMS and metastore fixtures and shared parser/traversal tests validate constructor and service generation, selectors, group descriptions, colon semantics, and coverage. #4956 retains all twelve Google runtime contracts; parent #4850 stays open for fresh full gcloud generation, generated options/services/API/provenance review, and integration validation. No generated integration file changes here.
Validation:
6dc510becab3d79893ce9f258499de067fd078a3, after fix(generator): preserve Terraform required option markers #4856 and test(distributed): trace cache concurrency timeout phases #5103. Preserved main's required-marker helper and both sets of API baseline additions. Other affected C# files originate from fix(generator): preserve gcloud resource arguments #4956 headb0f54576aec6b35fae7b3dea1e7ee0c4c24cd3fb. The CodeFactor follow-up extracts optional nested operand conversion into a separate helper without changing parsing behavior.git diff --checkpass.Closes #5112
Refs #4850