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: bump to Go 1.22 #375

Merged
merged 8 commits into from
Mar 10, 2024

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Mar 7, 2024

This repo is still on Go 1.19 which makes it difficult to develop simultaneous on this repo + celestia-app or celestia-core because they're already on Go 1.22. By difficult, I mean:

  1. go mod tidy will try to bump the Go version in this repo
  2. VSCode's Go language server (gopls?) breaks because it's running Go 1.22.1 on a Go 1.19 codebase

To resolve, this PR bumps the Go version, celestia-core version (to fix incompatible pyroscope deps), setup-go version in CI, golangci-lint because previous version wasn't compatible with Go 1.22. Then fixed lint issues reported by newer golangci-lint.

@rootulp rootulp self-assigned this Mar 7, 2024
Also checkout codebase before setup go
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.31%. Comparing base (0244d0c) to head (4950d8c).

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                      @@
##           release/v0.46.x-celestia     #375      +/-   ##
============================================================
- Coverage                     65.52%   64.31%   -1.21%     
============================================================
  Files                           666      761      +95     
  Lines                         71213    76379    +5166     
============================================================
+ Hits                          46662    49123    +2461     
- Misses                        21964    24481    +2517     
- Partials                       2587     2775     +188     

see 96 files with indirect coverage changes

Inspired by: cosmos@ac74e23
Fixes thislint error:

crypto/keyring/keyring.go:722:14: G306: Expect WriteFile permissions to be 0600 or less (gosec)
			if err := os.WriteFile(dir+"/keyhash", passwordHash, 0o555); err != nil {
Not used on cosmos-sdk main branch
@@ -363,6 +363,7 @@ func (k Keeper) PruneProposals(ctx sdk.Context) error {
return nil
}
for _, proposal := range proposals {
proposal := proposal
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolves this lint warning:

x/group/keeper/keeper.go:375:18: G601: Implicit memory aliasing in for loop. (gosec)
				TallyResult: &proposal.FinalTallyResult,

Inspired by the implementation on Cosmos SDK main

@@ -719,7 +719,7 @@ func newRealPrompt(dir string, buf io.Reader) func(string) (string, error) {
continue
}

if err := os.WriteFile(dir+"/keyhash", passwordHash, 0o555); err != nil {
if err := os.WriteFile(dir+"/keyhash", passwordHash, 0o600); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve this lint warning:

crypto/keyring/keyring.go:722:14: G306: Expect WriteFile permissions to be 0600 or less (gosec)
			if err := os.WriteFile(dir+"/keyhash", passwordHash, 0o555); err != nil {

Inspired by the implementation on Cosmos SDK main

@@ -6,7 +6,6 @@ run:
linters:
disable-all: true
enable:
- depguard
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this linter because it has a ton of warnings. To resolve I think we need to define every library that is acceptable to import in this repo.

The main branch of Cosmos SDK no longer uses this linter and I think it wasn't actually working on the previous golangci-lint version

@rootulp rootulp marked this pull request as ready for review March 8, 2024 18:44
@rootulp rootulp merged commit 2a2347c into celestiaorg:release/v0.46.x-celestia Mar 10, 2024
36 checks passed
@rootulp rootulp deleted the rp/bump-go branch March 10, 2024 19:50
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.

4 participants