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

Update GO.MOD Dependencies + Upgrade to GO1.19 #1020

Merged
merged 1 commit into from
Nov 9, 2022
Merged

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Nov 8, 2022

closes #956

I'm upgrading the dependency version because of dependabot. Unfortunately the change is huge and all linters started to report different types of errors. That's why I'm upgrading the linters, which in turn requires upgrading the GO version. With all these changes I had to make changes in the code and linters because of deprecated libraries and linters. I also had to make changes to our build pipelines.

Signed-off-by: kuritka kuritka@gmail.com

@kuritka kuritka changed the title Update references, GO1.19 Update GO.MOD Dependencies + Upgrade to GO1.19 Nov 8, 2022
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

Thanks for the tedious work.

What led to this nice ratio? Why go.sum became much shorter? Is it some go1.19 optimization?
image

@kuritka
Copy link
Collaborator Author

kuritka commented Nov 8, 2022

@ytsarev , yes, many dependencies throughout the dependency tree are away. With the generics, I guess they also switched to new versions of libraries that don't have so many internal dependencies. It also plays a role that I'm changing both go.sum files, not just one as usually.

ytsarev
ytsarev previously approved these changes Nov 8, 2022
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

cool, thanks for the clarification and the brave PR :)

Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka since this PR brings many potential changes, let's probably park this one until #1019 is sorted out.

@ytsarev
Copy link
Member

ytsarev commented Nov 9, 2022

@somaritane what do you mean by 'many potential changes' ? If terratest is green then we are covered or we do not trust our test suite?

I'm upgrading the dependency version because of dependabot. Unfortunately the change
is huge and all liners started to report different types of errors. That's why I'm upgrading
the liners, which in turn requires upgrading the GO version.
With all these changes I had to make changes in the code and linters because of deprecated
libraries and linters. I also had to make changes to our build pipelines.

Signed-off-by: kuritka <kuritka@gmail.com>
@netlify
Copy link

netlify bot commented Nov 9, 2022

Deploy Preview for k8gb-preview ready!

Name Link
🔨 Latest commit 3775df3
🔍 Latest deploy log https://app.netlify.com/sites/k8gb-preview/deploys/636bcbd6b6b80a0008d1e96d
😎 Deploy Preview https://deploy-preview-1020--k8gb-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@somaritane
Copy link
Contributor

@somaritane what do you mean by 'many potential changes' ? If terratest is green then we are covered or we do not trust our test suite?

@ytsarev I'm thinking more about potential rebase issues for other open PRs we have, but if @kuritka is OK to manage them, I'm OK too.

@somaritane
Copy link
Contributor

@kuritka we might need to update the docs for 1.19
https://github.com/k8gb-io/k8gb/blob/master/docs/local.md?plain=1#L25

@kuritka
Copy link
Collaborator Author

kuritka commented Nov 9, 2022

@somaritane , documentation change amended. There is a risk that dependabot will discover some CVE issue, because I radically cut the exclude and replace sections in the go.mod files. I've checked the versions and it should be fine, but it's possible I might have missed something or something new will come up. In that case, a fix PR on CVE will be done.

@somaritane
Copy link
Contributor

@somaritane , documentation change amended. There is a risk that dependabot will discover some CVE issue, because I radically cut the exclude and replace sections in the go.mod files. I've checked the versions and it should be fine, but it's possible I might have missed something or something new will come up. In that case, a fix PR on CVE will be done.

no worries @kuritka , by that time we might be running renovate instead :)

Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@kuritka kuritka merged commit e75ae31 into master Nov 9, 2022
@kuritka kuritka deleted the bump-dependencies branch November 9, 2022 19:46
nicsmith pushed a commit to nicsmith/k8gb that referenced this pull request Jan 14, 2023
New release of k3d-action solved the problem with importing images [k8gb-io#1020](k3d-io/k3d#1020).
We don't need to explicitly specify -mode=tools-node anymore, because this value is now the default.

Signed-off-by: kuritka <kuritka@gmail.com>
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.

GO 1.19 migration - terratests
3 participants