-
Notifications
You must be signed in to change notification settings - Fork 556
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
feat: add s3 batch result code constants #302
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #302 +/- ##
==========================================
- Coverage 74.60% 72.46% -2.15%
==========================================
Files 13 18 +5
Lines 638 730 +92
==========================================
+ Hits 476 529 +53
- Misses 123 136 +13
- Partials 39 65 +26 ☔ View full report in Codecov by Sentry. |
events/s3_batch_job.go
Outdated
ResultCode string `json:"resultCode"` | ||
ResultString string `json:"resultString"` | ||
TaskID string `json:"taskId"` | ||
ResultCode S3BatchJobResultCode `json:"resultCode"` |
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.
I think adding the type creates a breaking change. Assignment to a string will require a type cast
ex: https://play.golang.org/p/T5G1eKtbBor
I think leaving the type as string
is fine
// S3BatchJobResultCode represents the result of an S3BatchJobTask (i.e. | ||
// succeeded, permanent failure, or temmporary failure) | ||
const ( | ||
S3BatchJobResultCodeSucceeded S3BatchJobResultCode = "Succeeded" |
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.
downside of changing S3BatchJobResult.ResultCode
type back to string
, is that comparisons against this new type require a cast!
if inputEvent.Results[0].ResultCode == S3BatchJobResultCodeSucceeded {
// ..
}
->
Invalid operation: inputEvent.Results[0].ResultCode == S3BatchJobResultCodeSucceeded (mismatched types string and S3BatchJobResultCode)
Instead, go back to having S3BatchJobResult being a S3BatchJobResultCode
. It is useful for self documentation purposes. But also change the type to be a type alias eg: type S3BatchJobResultCoce = string
- this will preserve backwards compatibility.
This adds constants and a type for S3 Batch Result Codes similar to what
is available for CodeBuild results