-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: errors silently ignored when deleting resources #792
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
==========================================
+ Coverage 59.68% 59.73% +0.04%
==========================================
Files 210 210
Lines 7639 7641 +2
==========================================
+ Hits 4559 4564 +5
+ Misses 2437 2435 -2
+ Partials 643 642 -1 ☔ View full report in Codecov by Sentry. |
internal/cmd/base/delete.go
Outdated
for _, result := range results { | ||
if result.Error != nil { | ||
errs = append(errs, result.Error) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about combining this with the existing loop of results
in Line 95/100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would change the order of errors. Right now it's chronologic: errors that happen when starting the action come before errors that happened during the action. But I think you could move the loop above L94-98 to combine the loops without changing order
Can we add a test to detect this in the future? |
🤖 I have created a release *beep* *boop* --- ## [1.44.1](v1.44.0...v1.44.1) (2024-06-26) ### Bug Fixes * errors silently ignored when deleting resources ([#792](#792)) ([a7655f8](a7655f8)), closes [#791](#791) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
See #791 for a description of the issue.
Closes #791