fix: restore os.Args after plugin completion and fix error return#6817
fix: restore os.Args after plugin completion and fix error return#6817thaJeztah merged 1 commit intodocker:masterfrom
Conversation
thaJeztah
left a comment
There was a problem hiding this comment.
whoops! I started reviewing, but didn't submit 🙈
8e93ef7 to
402b1db
Compare
|
Updated in commit 402b1db and addressed all review points (inline parse error handling, helper returns error, stdlib temp file setup, and config patch simplification). I also resolved the review threads.\n\n@thaJeztah could you please take another look when you have a moment? Thanks! |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Signed-off-by: luojiyin <luojiyin@hotmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
402b1db to
6b1ba1a
Compare
There was a problem hiding this comment.
Pull request overview
Fixes two bugs in the plugin manager: a wrong error variable return on flag parse failure, and missing restoration of os.Args after plugin completion.
Changes:
- Fix error return in
RunEto use the inlineerrfromflags.Parseinstead of the shadowedperr - Save and restore
os.ArgswithdeferinValidArgsFunctionto prevent side effects - Add regression tests for both fixes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
cli-plugins/manager/cobra.go |
Fix error variable and add os.Args save/restore |
cli-plugins/manager/cobra_test.go |
Add tests for parse error return and os.Args restoration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- What I did
Fix two issues in the plugin manager:
- How I did it
errtoperrwhen flag parsing failsos.Argsbefore modification and usedeferto restore it- Files changed:
cli-plugins/manager/cobra.gocli-plugins/manager/cobra_test.go- How to verify it
The new tests verify both scenarios.
- Description for the changelog
Fixes #6816