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.
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.
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