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

chore: Extract (events, merkle) errors to errors.go #973

Merged
merged 3 commits into from
Dec 17, 2022

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Part of #257

Description

Extracts (events, merkle) errors to errors.go. Smaller PR as the next packages are net and node which are a bit larger RE errors and I dont want that to get too big.

@AndrewSisley AndrewSisley added area/errors Related to the internal management or design of our error handling system action/no-benchmark Skips the action that runs the benchmark. labels Dec 16, 2022
@AndrewSisley AndrewSisley added this to the DefraDB v0.4 milestone Dec 16, 2022
@AndrewSisley AndrewSisley requested a review from a team December 16, 2022 20:02
@AndrewSisley AndrewSisley self-assigned this Dec 16, 2022
@@ -104,7 +102,7 @@ func (mc *MerkleClock) AddDAGNode(
// write the delta and heads to a new block
nd, err := mc.putBlock(ctx, heads, height, delta)
if err != nil {
return nil, errors.Wrap("could not add block ", err)
return nil, err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dropped this wrap and the one at the one at the end as they are wrapping previously well defined errors local to this package and dont really seem to add any value

Copy link
Collaborator

Choose a reason for hiding this comment

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

good idea.

@AndrewSisley AndrewSisley force-pushed the sisley/chore/I257-extract-errors-4 branch from c05babb to 3f3120e Compare December 16, 2022 20:05
@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #973 (3f3120e) into develop (91b53f0) will decrease coverage by 0.10%.
The diff coverage is 5.55%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #973      +/-   ##
===========================================
- Coverage    57.82%   57.72%   -0.11%     
===========================================
  Files          169      170       +1     
  Lines        19420    19440      +20     
===========================================
- Hits         11229    11221       -8     
- Misses        7202     7226      +24     
- Partials       989      993       +4     
Impacted Files Coverage Δ
merkle/clock/errors.go 0.00% <0.00%> (ø)
merkle/clock/heads.go 70.00% <0.00%> (ø)
merkle/crdt/factory.go 85.71% <ø> (ø)
merkle/clock/clock.go 65.74% <10.00%> (+1.77%) ⬆️
events/simple.go 91.17% <100.00%> (ø)
net/client.go 78.04% <0.00%> (-4.88%) ⬇️
net/peer.go 44.01% <0.00%> (-2.16%) ⬇️
net/pb/net.pb.go 15.66% <0.00%> (+0.16%) ⬆️

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

Thanks for doing these. Looks good.

@AndrewSisley AndrewSisley merged commit ff13743 into develop Dec 17, 2022
@AndrewSisley AndrewSisley deleted the sisley/chore/I257-extract-errors-4 branch December 17, 2022 19:58
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Extract events errors to errors.go

* Extract clock errors to errors.go

* Extract crdt errors to errors.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/errors Related to the internal management or design of our error handling system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants