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

Updated dapper files #418

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Updated dapper files #418

merged 2 commits into from
Nov 28, 2022

Conversation

steffantucker
Copy link

@steffantucker steffantucker commented Jul 8, 2022 •
edited
Loading

Migrated to a SUSE container, a newer version of Go, and using golangci-lint for all linting. Fulfills rancher/tasks#203

@steffantucker steffantucker changed the title Updated dapper files [WIP] Updated dapper files Jul 8, 2022
@steffantucker steffantucker marked this pull request as draft July 8, 2022 16:22
@steffantucker steffantucker marked this pull request as ready for review July 11, 2022 22:50
@steffantucker steffantucker changed the title [WIP] Updated dapper files Updated dapper files Jul 11, 2022
Steffan Tucker added 2 commits July 26, 2022 10:40
Updated dapper files
e8b47ab 
Migrated to SUSE containers, a newer version of Go, and using golangci-lint for all linting.
Fix golangci-lint issues
1e41884 
As part of updating dapper files, golangci-lint was set to be used. This
caused a lot of lint issues to crop up, which this fixes.
@steffantucker steffantucker self-assigned this Aug 30, 2022
jiaqiluo
jiaqiluo approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

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

LGTM

@jiaqiluo jiaqiluo requested review from a team November 23, 2022 23:14
@jiaqiluo
Copy link
Member

jiaqiluo commented Nov 23, 2022 •
edited
Loading

hi @rancher/rancher-team-1-neo-dev @rancher/rancher-team-2-hostbusters-dev
can we get one more review on this PR?
after this PR is merged, I can then bump Go to 1.19, which is beneficial for supporting 1.25

@jiaqiluo jiaqiluo requested a review from superseb November 23, 2022 23:23
superseb
superseb approved these changes Nov 28, 2022
View reviewed changes
Copy link
Contributor

@superseb superseb left a comment

Choose a reason for hiding this comment

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

It needs another update round but based on your comment, this will be done in a next PR. Please change the registry.suse.com/bci/golang:1.17 image to an exact version so the update tooling can create PRs to use updates, e.g. registry.suse.com/bci/golang:1.19-18.38

@jiaqiluo
Copy link
Member

It needs another update round but based on your comment, this will be done in a next PR. Please change the registry.suse.com/bci/golang:1.17 image to an exact version so the update tooling can create PRs to use updates, e.g. registry.suse.com/bci/golang:1.19-18.38

Merging this PR. I am going to handle the changes in the following PR.

kinarashah
kinarashah approved these changes Nov 28, 2022
View reviewed changes
Copy link
Member

@kinarashah kinarashah left a comment

Choose a reason for hiding this comment

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

Merging this to unblock for 1.25 changes. @jiaqiluo I noticed that we are removing go vet here, could you check if it's enabled by default and if not, enable it in your next PR?
An example is here, https://github.com/rancher/rancher/pull/23448/files#diff-7b9967b524b30a7e62fef0e2d0827b63026239c22e6f1c34bfad57d138b26edfR5

@kinarashah kinarashah merged commit 19eed06 into rancher:master Nov 28, 2022
@jiaqiluo
Copy link
Member

go vet is enabled by default in golangci-lint, so there is no need to enable it
see https://golangci-lint.run/usage/linters/#:~:text=govet%C2%A0%E2%9A%99%EF%B8%8F,v1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kinarashah kinarashah kinarashah approved these changes

@jiaqiluo jiaqiluo jiaqiluo approved these changes

@superseb superseb superseb approved these changes

@MbolotSuse MbolotSuse Awaiting requested review from MbolotSuse

@cbron cbron Awaiting requested review from cbron

Assignees

@steffantucker steffantucker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steffantucker @jiaqiluo @superseb @kinarashah