fix(cmd): remove unreachable dead code in --extension validation#3839
fix(cmd): remove unreachable dead code in --extension validation#3839Ankitsinghsisodya wants to merge 1 commit into
Conversation
…devent format The check for the extension flag being valid only with the cloudevent format has been simplified by removing the redundant validation. This streamlines the code and clarifies the intended usage of the extension flag.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Removes redundant validation logic around the --extension flag when the invoke output format is not CloudEvents.
Changes:
- Deleted a duplicate conditional check that produced a second, slightly different error message for the same invalid state.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -175,9 +175,6 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err | |||
| if effectiveFormat != "cloudevent" { | |||
| return fmt.Errorf("--extension (-e) is only valid with cloudevents") | |||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3839 +/- ##
==========================================
+ Coverage 53.44% 53.46% +0.01%
==========================================
Files 200 200
Lines 23450 23447 -3
==========================================
+ Hits 12534 12535 +1
+ Misses 9662 9658 -4
Partials 1254 1254
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Problem
In
cmd/invoke.go, the--extensionvalidation block contains two consecutiveifchecks, but the second is permanently unreachable:If the first condition is true the function has already returned. By the time
execution reaches the second
if,effectiveFormatis guaranteed to equal"cloudevent", so the second condition is alwaysfalse. The duplicate errormessage is also inconsistently worded.
Fix
Remove the unreachable block. The single check at line 175 is sufficient and
already carries the correct error message.
Testing
go test ./cmd/...