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.

Sign up for GitHub

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

Jump to bottom

refactor: enable errorlint and refactor code #14110

Merged
merged 22 commits into from
Jul 4, 2024
Merged

refactor: enable errorlint and refactor code #14110

merged 22 commits into from
Jul 4, 2024

Conversation

hoanguyenkh
Copy link
Contributor

@hoanguyenkh hoanguyenkh commented Jun 14, 2024 •
edited
Loading

What type of PR is this?

Other

What does this PR do? Why is it needed?

  • Enable errorlint
  • Refactor code,
  • Remove directive errors comparison
  • Make fmt.Errorf use %w to wrap error instead of %v

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@hoanguyenkh hoanguyenkh requested a review from a team as a code owner June 14, 2024 09:36
@prestonvanloon prestonvanloon self-requested a review June 14, 2024 15:22
@CLAassistant
Copy link

CLAassistant commented Jun 15, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

terencechain
terencechain previously approved these changes Jun 16, 2024
View reviewed changes
@hoanguyenkh
Copy link
Contributor Author

hi @nalepae @saolyn can you merge PR?

prestonvanloon
prestonvanloon requested changes Jun 24, 2024
View reviewed changes
Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

Apologies for the eager approval. Upon closer review, there are some things that need to be addressed. Thanks

beacon-chain/node/registration/p2p.go Outdated Show resolved Hide resolved
beacon-chain/rpc/eth/beacon/handlers_validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/eth/shared/errors.go Outdated Show resolved Hide resolved
beacon-chain/sync/rpc.go Outdated Show resolved Hide resolved
beacon-chain/sync/rpc_beacon_blocks_by_range_test.go Outdated Show resolved Hide resolved
cmd/prysmctl/p2p/handler.go Outdated Show resolved Hide resolved
config/params/loader.go Outdated Show resolved Hide resolved
tools/replay-http/main.go Outdated Show resolved Hide resolved
validator/graffiti/parse_graffiti.go Outdated Show resolved Hide resolved
@prestonvanloon
Copy link
Member

You need to run gazelle on this PR.

bazel run //:gazelle

@hoanguyenkh
Copy link
Contributor Author

thank @prestonvanloon for feedback, please check it again! thank you

prestonvanloon
prestonvanloon approved these changes Jun 26, 2024
View reviewed changes
Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!!

@hoanguyenkh hoanguyenkh disabled auto-merge July 1, 2024 14:04
@hoanguyenkh
Copy link
Contributor Author

can you help me merge it @prestonvanloon

@rkapka rkapka enabled auto-merge July 4, 2024 21:35
@rkapka rkapka added this pull request to the merge queue Jul 4, 2024
Merged via the queue into prysmaticlabs:develop with commit 7a39406 Jul 4, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prestonvanloon prestonvanloon prestonvanloon approved these changes

@terencechain terencechain terencechain left review comments

@nalepae nalepae Awaiting requested review from nalepae nalepae is a code owner automatically assigned from prysmaticlabs/core-team

@saolyn saolyn Awaiting requested review from saolyn saolyn is a code owner automatically assigned from prysmaticlabs/core-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hoanguyenkh @CLAassistant @prestonvanloon @terencechain @rkapka