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(dot/sync): handleJustification returns errors #2810

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Sep 8, 2022

Changes

  • Remove nil check on header and justification (unused, see caller checks - and caller should be the ones checking what they pass down)
  • Return wrapped errors to callers
  • Change callers to handle non-nil errors and return them up the call stack
  • Adapt tests

Tests

go test -tags integration github.com/ChainSafe/gossamer/dot/sync/...

Issues

Primary Reviewer

@noot

@qdm12 qdm12 force-pushed the qdm12/dot/sync/handle-just branch from 75b66e8 to 9ff513c Compare September 8, 2022 15:40
@qdm12 qdm12 force-pushed the qdm12/genesis/less-dependencies branch from d5eed9a to 95ba320 Compare September 12, 2022 19:39
@qdm12 qdm12 closed this Sep 12, 2022
@qdm12 qdm12 force-pushed the qdm12/dot/sync/handle-just branch from 9ff513c to 95ba320 Compare September 12, 2022 20:18
@qdm12 qdm12 reopened this Sep 12, 2022
if len(justification) == 0 {
return nil
}
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 don't think that can ever happen really, but keeping it to be safe ™️

@qdm12 qdm12 force-pushed the qdm12/genesis/less-dependencies branch 2 times, most recently from f155179 to edd1f99 Compare September 16, 2022 12:29
@qdm12 qdm12 force-pushed the qdm12/dot/sync/handle-just branch from ceb798e to 1d661d6 Compare September 16, 2022 12:29
@qdm12 qdm12 force-pushed the qdm12/genesis/less-dependencies branch 2 times, most recently from 357cabe to 6b0f0eb Compare September 20, 2022 14:39
Base automatically changed from qdm12/genesis/less-dependencies to development September 20, 2022 15:14
@qdm12 qdm12 force-pushed the qdm12/dot/sync/handle-just branch from 1d661d6 to 793bebc Compare September 20, 2022 15:17
@qdm12 qdm12 added the PR Easy label Sep 20, 2022
@qdm12 qdm12 marked this pull request as ready for review September 20, 2022 15:17
@qdm12 qdm12 force-pushed the qdm12/dot/sync/handle-just branch from 793bebc to 7707152 Compare September 22, 2022 17:40
@qdm12 qdm12 merged commit 37fda37 into development Sep 22, 2022
@qdm12 qdm12 deleted the qdm12/dot/sync/handle-just branch September 22, 2022 18:15
@github-actions
Copy link

🎉 This PR is included in version 0.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants