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

Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 #80

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 #80

merged 2 commits into from
Jun 4, 2024

Conversation

thepetk
Copy link
Contributor

@thepetk thepetk commented Jun 4, 2024 •
edited
Loading

What does this PR do?

Bumps up the go version of alizer to 1.21 and k8s.io/apimachinery to v0.29.2 as part of the general EPIC devfile/api#1555

Which issue(s) does this PR fix

Fixes devfile/api#1560

PR acceptance criteria

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Unit/Functional tests

  • Documentation

How to test changes / Special notes to the reviewer

@openshift-ci openshift-ci bot added the approved label Jun 4, 2024
@thepetk thepetk changed the title Bump up go version to 1.21 Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 Jun 4, 2024
Jdubrick
Jdubrick approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@Jdubrick Jdubrick left a comment

Choose a reason for hiding this comment

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

/lgtm

Everything seemed to work locally with building the binaries and running tests. Go is backwards compatible so I don't see any reason this would give us a headache and it looks like the api version was updated indirectly anyway?

@openshift-ci openshift-ci bot added the lgtm label Jun 4, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jdubrick, thepetk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thepetk
Copy link
Contributor Author

thepetk commented Jun 4, 2024

/lgtm

Everything seemed to work locally with building the binaries and running tests. Go is backwards compatible so I don't see any reason this would give us a headache and it looks like the api version was updated indirectly anyway?

@Jdubrick thanks for the review :) Yeap the update of the API comes from the update of:

sigs.k8s.io/controller-runtime v0.17.5

Which reminds me we might want to get rid of zap because is the only reason we are depending on runtime.

@Jdubrick
Copy link
Contributor

Jdubrick commented Jun 4, 2024

/lgtm
Everything seemed to work locally with building the binaries and running tests. Go is backwards compatible so I don't see any reason this would give us a headache and it looks like the api version was updated indirectly anyway?

@Jdubrick thanks for the review :) Yeap the update of the API comes from the update of:

sigs.k8s.io/controller-runtime v0.17.5

Which reminds me we might want to get rid of zap because is the only reason we are depending on runtime.

It looks like we are using that for a logger though, no?

@thepetk
Copy link
Contributor Author

thepetk commented Jun 4, 2024

/lgtm
Everything seemed to work locally with building the binaries and running tests. Go is backwards compatible so I don't see any reason this would give us a headache and it looks like the api version was updated indirectly anyway?

@Jdubrick thanks for the review :) Yeap the update of the API comes from the update of:

sigs.k8s.io/controller-runtime v0.17.5

Which reminds me we might want to get rid of zap because is the only reason we are depending on runtime.

It looks like we are using that for a logger though, no?

Yeah I guess we should use another dependency to handle logging.

@thepetk thepetk merged commit 76bc4ff into devfile:main Jun 4, 2024
5 checks passed
@thepetk
Copy link
Contributor Author

thepetk commented Jun 4, 2024

btw I've merged it as the issue we're discussing it's out of scope :)

@Jdubrick
Copy link
Contributor

Jdubrick commented Jun 4, 2024

btw I've merged it as the issue we're discussing it's out of scope :)

Sounds good. We should open an issue to investigate if there is a better way to handle logging if we want to remove that as a dependency?

@thepetk
Copy link
Contributor Author

thepetk commented Jun 4, 2024

btw I've merged it as the issue we're discussing it's out of scope :)

Sounds good. We should open an issue to investigate if there is a better way to handle logging if we want to remove that as a dependency?

Yup! I'm already on it

thepetk added a commit to thepetk/devfile-alizer that referenced this pull request Jun 17, 2024
@thepetk
Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 (devfil…
1212ce6 
…e#80)

* Bump up go to 1.21 and apimachinery to 0.29.2

Signed-off-by: thepetk <[email protected]>

* Update workflows and documentation

Signed-off-by: thepetk <[email protected]>

---------

Signed-off-by: thepetk <[email protected]>
thepetk added a commit to thepetk/devfile-alizer that referenced this pull request Jun 17, 2024
@thepetk
Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 (devfil…
337fc6b 
…e#80)

* Bump up go to 1.21 and apimachinery to 0.29.2

Signed-off-by: thepetk <[email protected]>

* Update workflows and documentation

Signed-off-by: thepetk <[email protected]>

---------

Signed-off-by: thepetk <[email protected]>
thepetk added a commit to thepetk/devfile-alizer that referenced this pull request Jun 17, 2024
@thepetk
Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 (devfil…
042134f 
…e#80)

* Bump up go to 1.21 and apimachinery to 0.29.2

Signed-off-by: thepetk <[email protected]>

* Update workflows and documentation

Signed-off-by: thepetk <[email protected]>

---------

Signed-off-by: thepetk <[email protected]>
thepetk added a commit that referenced this pull request Jun 17, 2024
@thepetk @michael-valdron
Update workflows to point to go.mod (#84)
42574ef 
* Update workflows

Signed-off-by: thepetk <[email protected]>

* Bump up go version to 1.21 and k8s.io/apimachinery to v0.29.2 (#80)

* Bump up go to 1.21 and apimachinery to 0.29.2

Signed-off-by: thepetk <[email protected]>

* Update workflows and documentation

Signed-off-by: thepetk <[email protected]>

---------

Signed-off-by: thepetk <[email protected]>

* Update .github/workflows/CI.yml

Co-authored-by: Michael Valdron <[email protected]>
Signed-off-by: thepetk <[email protected]>

* Update .github/workflows/check_registry.yaml

Co-authored-by: Michael Valdron <[email protected]>
Signed-off-by: thepetk <[email protected]>

* Update .github/workflows/release.yaml

Co-authored-by: Michael Valdron <[email protected]>
Signed-off-by: thepetk <[email protected]>

* Update .github/workflows/CI.yml

Co-authored-by: Michael Valdron <[email protected]>
Signed-off-by: thepetk <[email protected]>

* Revert formatting

Signed-off-by: thepetk <[email protected]>

---------

Signed-off-by: thepetk <[email protected]>
Co-authored-by: Michael Valdron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jdubrick Jdubrick Jdubrick approved these changes

@elsony elsony Awaiting requested review from elsony

@michael-valdron michael-valdron Awaiting requested review from michael-valdron

Assignees

@Jdubrick Jdubrick

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Alizer should be updated to support Go 1.21
2 participants
@thepetk @Jdubrick