-
Notifications
You must be signed in to change notification settings - Fork 39
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(claim.go): add Outputs field to Claim struct #66
Conversation
vdice
commented
Jul 19, 2019
- add Outputs field to Claim struct, per cnab-spec (updated in Add outputs to claim spec cnab-spec#217)
153a818
to
395b85a
Compare
claim/claimstore_test.go
Outdated
"foo-output": true, | ||
"bar-output": "bar", | ||
} | ||
is.Equal(c.Outputs, want, "Wrong outputs on claim") |
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.
The signature for Equal is first the expected result, then the actual result. This got switched here which can lead to confusing error messages if the test were to fail.
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.
Thank you! Fixed.
claim/claimstore_test.go
Outdated
"foo-output": true, | ||
"bar-output": "baz", | ||
} | ||
is.Equal(c.Outputs, want, "Wrong outputs on claim") |
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.
Same here, the parameter order should be reversed.
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.
Thank you! Fixed.
395b85a
to
33b7052
Compare
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.
LGTM
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.
LGTM