Skip to content
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/Support signature status #405

Merged
merged 1 commit into from
Jun 30, 2022
Merged

Feat/Support signature status #405

merged 1 commit into from
Jun 30, 2022

Conversation

carpawell
Copy link
Member

@carpawell carpawell commented Jun 16, 2022

@carpawell carpawell added the enhancement Improving existing functionality label Jun 16, 2022
@carpawell carpawell self-assigned this Jun 16, 2022
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #405 (e990a97) into master (c6f7ab3) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
- Coverage   63.88%   63.84%   -0.05%     
==========================================
  Files          75       75              
  Lines        9182     9182              
==========================================
- Hits         5866     5862       -4     
- Misses       2612     2614       +2     
- Partials      704      706       +2     
Impacted Files Coverage Δ
status/status.go 0.00% <ø> (ø)
object/marshal.go 73.99% <0.00%> (-0.74%) ⬇️
object/convert.go 75.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6f7ab3...e990a97. Read the comment docs.

Comment on lines 23 to 27
// ErrUnsupportedSignatureScheme is returned when
// the library does not support a signature scheme
// that was used for signing a data.
type ErrUnsupportedSignatureScheme struct {
scheme refs.SignatureScheme
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it related to this commit?

Copy link
Member Author

@carpawell carpawell Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR adds supporting signature status code and adds some functionality that allows api-go callers to understand what exactly error happened (to use the new status code)

or do you just mean to put that to a separate commit?

please, see the node PR decs before merging that PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should either create a separate API status or continue using a string message with all wrapping context.
This type just adds another level of abstraction, api-go should be small and reflect API as much as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dropped

@carpawell carpawell requested a review from fyrchik June 29, 2022 07:22
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell merged commit bb9fc5b into nspcc-dev:master Jun 30, 2022
@carpawell carpawell deleted the feat/add-signature-related-status branch June 30, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants