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

Upgrade go version to 1.20 #1246

Merged
merged 7 commits into from
Mar 1, 2023
Merged

Upgrade go version to 1.20 #1246

merged 7 commits into from
Mar 1, 2023

Conversation

letmerecall
Copy link
Contributor

Description

For #1243

To upgrade go version to 1.20.

Issue reference

Please reference the issue this PR will close: #1243

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@letmerecall letmerecall requested review from a team as code owners February 24, 2023 14:00
@mukundansundar
Copy link
Collaborator

mukundansundar commented Feb 25, 2023

@letmerecall can you run make modtidy and fix the dependencies in CLI?

@letmerecall
Copy link
Contributor Author

letmerecall commented Feb 25, 2023

@mukundansundar make modtidy does nothing. It looks like the dependencies are alright. However, looking into the workflow logs, it seems that the job is picking up GOVER: 1.2 (probably from cache) and we might have to add patch version to get through this.

Screenshot 2023-02-25 at 1 13 33 PM

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@letmerecall
Copy link
Contributor Author

@mukundansundar There's no apparent error in the logs of Build linux_amd64 binaries job but the runner received a shutdown signal. Should we try to re-run it?

@pravinpushkar
Copy link
Contributor

@letmerecall The linting step is failing in workflow. You should be able to run locally make lint with current lint version( version: v1.49.0) and go version(1.20.1) and check failures.

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@letmerecall
Copy link
Contributor Author

@letmerecall The linting step is failing in workflow. You should be able to run locally make lint with current lint version( version: v1.49.0) and go version(1.20.1) and check failures.

It's strange that I'm not able to run make lint locally with current lint version( version: v1.49.0). The command consumes a ton of resources before exiting with this message:

golangci-lint run --timeout=20m
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
WARN [runner] The linter 'ifshort' is deprecated (since v1.48.0) due to: The repository of the linter has been deprecated by the owner.  
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
make: *** [lint] Killed: 9

@pravinpushkar
Copy link
Contributor

It's strange that I'm not able to run make lint locally with current lint version( version: v1.49.0). The command consumes a ton of resources before exiting with this message:

We might need to upgrade golanglint to v1.51.0. Related to this -

golangci/golangci-lint#3414
https://github.com/golangci/golangci-lint/commits/v1.51.0

FYI @mukundansundar

@mukundansundar
Copy link
Collaborator

It's strange that I'm not able to run make lint locally with current lint version( version: v1.49.0). The command consumes a ton of resources before exiting with this message:

We might need to upgrade golanglint to v1.51.0. Related to this -

golangci/golangci-lint#3414

https://github.com/golangci/golangci-lint/commits/v1.51.0

FYI @mukundansundar

Will create a PR to upgrade linter version.

@mukundansundar
Copy link
Collaborator

It's strange that I'm not able to run make lint locally with current lint version( version: v1.49.0). The command consumes a ton of resources before exiting with this message:

We might need to upgrade golanglint to v1.51.0. Related to this -
golangci/golangci-lint#3414
https://github.com/golangci/golangci-lint/commits/v1.51.0
FYI @mukundansundar

Will create a PR to upgrade linter version.

will merge #1253 and then update this PR again.

Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

@letmerecall
Similar to https://github.com/dapr/dapr/pull/5876/files#diff-aed9855ec5ff916d6ced3a125bd9b76551a22febf05b7e6d4bafd9222eb26dfdR224-R228
Please setup go from the go.mod file if possible.

with:
           go-version-file: 'go.mod'

@mukundansundar
Copy link
Collaborator

@letmerecall can you please fix the conflicts

Signed-off-by: Girish Sharma <girishsharma001@gmail.com>
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #1246 (9ec7e97) into master (398eb7e) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #1246   +/-   ##
=======================================
  Coverage   27.35%   27.36%           
=======================================
  Files          38       38           
  Lines        3699     3698    -1     
=======================================
  Hits         1012     1012           
+ Misses       2621     2620    -1     
  Partials       66       66           
Impacted Files Coverage Δ
pkg/standalone/uninstall.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@mukundansundar
Copy link
Collaborator

mukundansundar commented Mar 1, 2023

@letmerecall Please checkout the code first and then run go setup action since the change uses go.mod for version value

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@letmerecall
Copy link
Contributor Author

letmerecall commented Mar 1, 2023

@letmerecall Please checkout the code first and then run go setup action since the change uses go.mod for version value

Oops, sorry.

I wonder if we have to clear the workflow cache as well? Last time setup-go@v3 action picked go version 1.2 instead of 1.20

@mukundansundar
Copy link
Collaborator

@letmerecall Please checkout the code first and then run go setup action since the change uses go.mod for version value

Oops, sorry.

I wonder if we have to clear the workflow cache as well? Last time setup-go@v3 action picked go version 1.2 instead of 1.20

@letmerecall I will take a look at this tomorrow morning and will work on fixing and merging this PR.

Signed-off-by: letmerecall <girishsharma001@gmail.com>
@letmerecall
Copy link
Contributor Author

letmerecall commented Mar 1, 2023

@letmerecall I will take a look at this tomorrow morning and will work on fixing and merging this PR.

@mukundansundar Cool! my last commit might fix the paths for 'go.mod' in workflow. Feel free to revert otherwise.

Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

lgtm.

@mukundansundar
Copy link
Collaborator

@letmerecall Thanks for the contribution !!

@mukundansundar mukundansundar merged commit 17124eb into dapr:master Mar 1, 2023
@letmerecall letmerecall deleted the update-go-version branch March 2, 2023 18: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.

Update to Go 1.20
3 participants