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

Move some errors to separate consensus-types package #12329

Merged
merged 5 commits into from
Apr 25, 2023

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Apr 25, 2023

What type of PR is this?

Cleanup

What does this PR do? Why is it needed?

ErrNilObjectWrapped, ErrUnsupportedGetter and ErrNotSupported all currently live in the blocks package, but they are applicable to all consensus types. This PR moves these members out to the top-level consensus-types package.

@rkapka rkapka added Ready For Review A pull request ready for code review OK to merge Cleanup Code health! labels Apr 25, 2023
@rkapka rkapka requested a review from a team as a code owner April 25, 2023 13:29
@rkapka rkapka marked this pull request as draft April 25, 2023 13:39
prestonvanloon
prestonvanloon previously approved these changes Apr 25, 2023
@rkapka rkapka marked this pull request as ready for review April 25, 2023 13:46
@rkapka rkapka changed the title Move ErrNilObjectWrapped to separate package Move some errors to separate consensus-types package Apr 25, 2023
@@ -5,6 +5,7 @@ import (

"github.com/pkg/errors"
ssz "github.com/prysmaticlabs/fastssz"
consensus_types "github.com/prysmaticlabs/prysm/v4/consensus-types"
Copy link
Contributor

@saolyn saolyn Apr 25, 2023

Choose a reason for hiding this comment

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

Should we not be using camel case instead of snake case? My IDE complains
Or alternatively just consensustypes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally it should me consensustypes. But Goland's intellisense uses snaka case and it's very annoying to convert every file manually, so I just leave it as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha

@prylabs-bulldozer prylabs-bulldozer bot merged commit 328e6fb into develop Apr 25, 2023
@delete-merged-branch delete-merged-branch bot deleted the err-nil-object-wrapped branch April 25, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health! Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants