feat(x/ecocredit): implement MsgMintBatch and MsgSealBatch interfaces #991
Conversation
|
@robert-zaremba is this ready for review? |
Yes. I've just turned it off draft and pushed the tests. |
| {"invalid issuer", MsgMintBatchCredits{Issuer: "invalid"}, "issuer"}, | ||
| {"invalid denom", MsgMintBatchCredits{Issuer: issuer, BatchDenom: "XXX"}, "invalid denom"}, | ||
| {"invalid note", | ||
| MsgMintBatchCredits{Issuer: issuer, BatchDenom: batchDenom, Note: randstr.String(514)}, "note must"}, | ||
| {"missing origin tx", | ||
| MsgMintBatchCredits{Issuer: issuer, BatchDenom: batchDenom}, "origin_tx is required"}, | ||
|
|
||
| {"good-no-note", MsgMintBatchCredits{Issuer: issuer, BatchDenom: batchDenom, OriginTx: &batchOrigTx, Issuance: batchIssuances}, ""}, | ||
| {"good-note", MsgMintBatchCredits{Issuer: issuer, BatchDenom: batchDenom, OriginTx: &batchOrigTx, Note: randstr.String(300), | ||
| Issuance: batchIssuances}, ""}, |
There was a problem hiding this comment.
Might be nice to give each field its own line to help with readability but ok to leave as is.
There was a problem hiding this comment.
Do you think it's not readable? I find it easier to read if it doesn't consume significant amount of vertical space.
There was a problem hiding this comment.
It is readable but I do find it easier to read if each field is on its own line and does not go beyond the line width. As mentioned above, ok to leave as is.
{"good-no-note", MsgMintBatchCredits{Issuer: issuer, BatchDenom: batchDenom, OriginTx: &batchOrigTx, Issuance: batchIssuances}, ""},
vs
{
"good-no-note",
MsgMintBatchCredits{
Issuer: issuer,
BatchDenom: batchDenom,
OriginTx: &batchOrigTx,
Issuance: batchIssuances,
},
"",
},
There was a problem hiding this comment.
stack many of them and your test will take multiple of screens.
| var errBadReq = sdkerrors.ErrInvalidRequest | ||
|
|
There was a problem hiding this comment.
Is this necessary? I would prefer to see sdkerrors.ErrInvalidRequest used directly.
There was a problem hiding this comment.
shorter lines are easier to read and don't require to break lines.
There was a problem hiding this comment.
I agree that shorter lines are easier to read. Curious why this applies here but not above. I still think there is more semantic value in using the sdk error directly given the amount of space saved here is minimal. Ok to leave as is though.
|
Also looks like you have a minor issue with imports: |
ryanchristo
changed the title
robert-zaremba
requested review from
technicallyty,
aleem1314,
aaronc,
anilcse,
clevinson,
ryanchristo and
blushi
|
Looks like you missed an import: |
Description
Closes: #990
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change