chore: got rid of most panics in share pkg #1512
Conversation
|
|
Does this close #1490 ? |
| @@ -182,6 +185,10 @@ func MinDataAvailabilityHeader() DataAvailabilityHeader { | |||
| } | |||
|
|
|||
| // MinShares returns one tail-padded share. | |||
| func MinShares() [][]byte { | |||
| return shares.ToBytes(shares.TailPaddingShares(appconsts.MinShareCount)) | |||
| func MinShares() ([][]byte, error) { | |||
There was a problem hiding this comment.
[nice to have, but optional]
we could add some documentation here to when an error is thrown to avoid having to dig through the tailpadding shares function.
There was a problem hiding this comment.
[no change needed] on re-review it seems strange to return an error from this function because there are no function parameters. I expect MinShares() to never return an error. It could panic() if shares.TailPaddingShares(appconsts.MinShareCount) returns an error but I don't think such a change is blocking.
Codecov Report
@@ Coverage Diff @@
## main #1512 +/- ##
==========================================
- Coverage 50.70% 49.90% -0.81%
==========================================
Files 82 82
Lines 4723 4813 +90
==========================================
+ Hits 2395 2402 +7
- Misses 2134 2193 +59
- Partials 194 218 +24
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -182,6 +185,10 @@ func MinDataAvailabilityHeader() DataAvailabilityHeader { | |||
| } | |||
|
|
|||
| // MinShares returns one tail-padded share. | |||
| func MinShares() [][]byte { | |||
| return shares.ToBytes(shares.TailPaddingShares(appconsts.MinShareCount)) | |||
| func MinShares() ([][]byte, error) { | |||
There was a problem hiding this comment.
[no change needed] on re-review it seems strange to return an error from this function because there are no function parameters. I expect MinShares() to never return an error. It could panic() if shares.TailPaddingShares(appconsts.MinShareCount) returns an error but I don't think such a change is blocking.
Context here: #1490
closes #1490
Overview
Checklist