Apply Liquibase CLI regeneration to main - #4044
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Liquibase scraper now preserves positional command operands and extracts stable versions through ChangesLiquibase CLI support
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to This PR regenerates Liquibase CLI options and updates generator behavior; targeted scraper and OptionsGenerator validations pass, but the required format check and Liquibase unit tests still need to run before merge, leaving readiness pending. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review
Reviewed this against the full prior review thread on #4026 (three rounds of claude/greptile feedback flagging: a corrupted toolVersion banner-scrape, ~254 spurious [Obsolete] markers from that corruption, and an uncompensated breaking rename of the Changeset→ChangeSet casing). This PR cherry-picks only the final, fixed squash commit from #4026 onto current main, and the diff here reflects the fully-resolved state:
toolVersionis now a real value ("5.0.3"inLiquibase.CommandCoverage.json), not the ASCII banner that plagued earlier revisions.- The
Changeset→ChangeSetcasing rename ships with compat shims everywhere it needs them:ILiquibase/Liquibase.Generated.csaddMarkNextChangeSetRan(Sql)Asyncwhile keeping[Obsolete]-forwarding overloads forMarkNextChangesetRan(Sql)Async;LiquibaseOptions.Generated.csandLiquibaseCalculateChecksumOptions.Generated.csdo the same for the renamed properties (AllowDuplicatedChangeSetIdentifiers,ChangeSetAuthor, etc.), and the options record type itself (LiquibaseMarkNextChangesetRanOptions) was deliberately left un-renamed to avoid a second breaking change. This matches the pattern already used elsewhere in this same generated surface for removed CLI options. - The merge conflict was resolved cleanly: the only integration conflict (
ParseLiquibaseVersionvs.ParseVersiontest helper naming) now has a singleParseVersionhelper, and there are no leftover call sites of the old name (verified via grep across the test file). - The
=> throw new NotSupportedException()default bodies added to everyILiquibaseinterface method are not new/risky behavior — this is an existing repo-wide generator convention already present inIDocker.Generated.csand other tool interfaces onmain; Liquibase is just catching up to it, not introducing it. - The scraper's new
ParseCommandAsync(..., UsageSynopsisParseResult, ...)overload andGetPositionalArgumentsusage mirror the exact pattern already used byDotNetCliScraper,BrewCliScraper,PnpmCliScraper,WinGetCliScraper, etc., confirming the stated prerequisite (#4003's positional-operand parsing) is indeed already onmainand this isn't inventing new generator mechanics.
No outstanding issues from the #4026 review thread remain in this diff, and I didn't find new problems in the cherry-picked changes themselves. The PR's self-reported validation (scraper/generator unit tests, clean diff-check) lines up with what's in the diff.
One minor, non-blocking note: the docs change to liquibase.md's module example (dropping the full runnable RunCommandModule sample for var liquibase = context.Tools.Liquibase;) is a documentation regression in isolation, but since this is a straight cherry-pick of an already-reviewed commit rather than new work, it's not this PR's place to fix — worth a follow-up if the shorter example is judged less useful than the original.
Looks good to merge.
GitHub Actions
Error
Greptile SummaryThis PR applies the previously reviewed Liquibase CLI regeneration to the current default branch.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/LiquibaseCliScraper.cs | Preserves positional operands from parsed Liquibase usage data and supports the regenerated command model. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Generators/GlobalOptionsBaseGenerator.cs | Changes compatibility-property emission so legacy virtual overrides remain effective through canonical generated properties. |
| src/ModularPipelines.Liquibase/Options/LiquibaseOptions.Generated.cs | Regenerates global options with corrected names and obsolete compatibility backing properties. |
| src/ModularPipelines.Liquibase/Services/ILiquibase.Generated.cs | Regenerates the Liquibase service contract, including renamed command aliases and default interface implementations. |
| src/ModularPipelines.Liquibase/Services/Liquibase.Generated.cs | Regenerates concrete command implementations and compatibility forwarding methods. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/LiquibaseCliScraperTests.cs | Adds regression coverage for version parsing and preservation of usage operands. |
Reviews (2): Last reviewed commit: "fix(generator): preserve virtual aliases" | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef6b5a7e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed the virtual-override compatibility issue at exact head 691bff1. The generator now preserves legacy virtual dispatch for direct singleton global-option aliases, and Liquibase was regenerated from CLI 5.0.3 (43 commands, 61 files; stable regeneration). Validation: OptionsGenerator tests 930/930; OptionsGenerator Release build 0 warnings/errors. Added generator and runtime regressions. The focused Liquibase test project reached the agent guard's 2 GB limit (2097 MB), so I did not raise the limit or retry; CI owns that check. |
GitHub Actions
Inactive
1 participant
Carries the reviewed Liquibase regeneration from #4026 onto the actual default branch. #4026 was merged into the obsolete stacked base after that base PR had already landed, so its squash commit never reached main.
This cherry-picks only the #4026 squash diff onto current main. The sole integration conflict was two names for the same Liquibase version-parser test helper; both regressions now share one helper. No generated file was hand-edited.
Validation:
The prior full Liquibase solution build reached the fixed 2 GB agent guard, so it was not retried or raised; CI owns that expensive validation.
Refs #4026
Refs #3996
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests