Skip to content

feat: changed geth library#683

Merged
aloknerurkar merged 4 commits intomainfrom
fix/geth-lib-update
May 9, 2025
Merged

feat: changed geth library#683
aloknerurkar merged 4 commits intomainfrom
fix/geth-lib-update

Conversation

@Mikelle
Copy link
Copy Markdown
Contributor

@Mikelle Mikelle commented May 9, 2025

Describe your changes

  • Updating geth library
  • Moved to the new go linter version

Checklist before requesting a review

  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation

@Mikelle Mikelle requested review from aloknerurkar and shaspitz May 9, 2025 14:35
@Mikelle Mikelle self-assigned this May 9, 2025
@Mikelle Mikelle force-pushed the fix/geth-lib-update branch from 7c60af9 to 1e9aa3e Compare May 9, 2025 14:46
Comment thread bridge/standard/cmd/relayer/main.go Outdated
signal.Notify(sigc, syscall.SIGINT, syscall.SIGTERM)
go func() {
<-sigc
//nolint:errcheck
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's enough //nolint:errcheck in this PR that we should disable that check imo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we don't have configuration for linter: .golangci.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we create one? I don't see the use of adding tens to hundreds of lint exceptions in a PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't specify, which errcheck we should ignore. I think it's still useful, just not for the .Close functions and fmt.Printf

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of adding the lint ignore we can just add:

_ = fmt.Fprintln()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@aloknerurkar added placeholders

Comment thread bridge/standard/cmd/relayer/main.go Outdated
signal.Notify(sigc, syscall.SIGINT, syscall.SIGTERM)
go func() {
<-sigc
//nolint:errcheck
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of adding the lint ignore we can just add:

_ = fmt.Fprintln()

Comment thread bridge/standard/go.mod
go 1.23
go 1.23.0

toolchain go1.24.0
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this added?? Why is it using a higher version?

Copy link
Copy Markdown
Contributor Author

@Mikelle Mikelle May 9, 2025

Choose a reason for hiding this comment

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

it was done automatically after geth lib update and go mod tidy

@Mikelle Mikelle requested review from aloknerurkar and shaspitz May 9, 2025 19:17
@aloknerurkar aloknerurkar merged commit 53aa617 into main May 9, 2025
4 of 5 checks passed
@aloknerurkar aloknerurkar deleted the fix/geth-lib-update branch May 9, 2025 19:53
aloknerurkar pushed a commit that referenced this pull request May 9, 2025
Mikelle added a commit that referenced this pull request May 9, 2025
Mikelle added a commit that referenced this pull request May 9, 2025
aloknerurkar pushed a commit that referenced this pull request May 9, 2025
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