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

Migrations to golang 1.19 #271

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kmlebedev
Copy link

What this PR does / why we need it:

migrations to golang 1.19

@kmlebedev kmlebedev changed the title initial migrations to golang 1.19 Sep 26, 2022
@kmlebedev kmlebedev changed the title migrations to golang 1.19 Migrations to golang 1.19 Sep 26, 2022
@kmlebedev
Copy link
Author

Copy link
Collaborator

@maiqueb maiqueb left a comment

Choose a reason for hiding this comment

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

It's confusing to see the PR structured as is: multiple times a commit is just fixing something which was broken in a previous commit.

This makes the reviewers life harder, and renders tools like git bisect useless if this happens to be merged.

Could you please squash everything into a single commit - or structure the commits in a better way ?

Finally, I wonder if we should jump to 1.19 now. I personally would only do it once 1.18 goes EOL - meaning, I think the project's go.mod should use the oldest supported version of golang.

.github/workflows/build.yml Show resolved Hide resolved
e2e/client/ippool.go Show resolved Hide resolved
pkg/storage/kubernetes/ipam.go Show resolved Hide resolved
e2e/client/ippool.go Show resolved Hide resolved
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3188704025

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.659%

Totals Coverage Status
Change from base Build 3159637312: 0.0%
Covered Lines: 900
Relevant Lines: 1292

💛 - Coveralls

@dougbtv
Copy link
Member

dougbtv commented Dec 5, 2022

@kmlebedev any chance you can rebase? We can take another look at moving to 1.19 thanks for the PR

@pjbgf pjbgf mentioned this pull request Jan 17, 2023
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