Skip to content

Bump go to v1.17 #922

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
Merged
merged 1 commit into from
Feb 23, 2022
Merged

Bump go to v1.17 #922

merged 1 commit into from
Feb 23, 2022

Conversation

@awgreene
Copy link
Member

@awgreene awgreene commented Feb 22, 2022

No description provided.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 22, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2022
@codecov
Copy link

codecov bot commented Feb 22, 2022 •
edited
Loading

Codecov Report

Merging #922 (e06edc2) into master (c496cf0) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
- Coverage   52.21%   52.19%   -0.02%     
==========================================
  Files         103      103              
  Lines        9117     9117              
==========================================
- Hits         4760     4759       -1     
- Misses       3451     3452       +1     
  Partials      906      906
Impacted Files Coverage Δ
pkg/registry/query.go 60.41% <0.00%> (-0.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c496cf0...e06edc2. Read the comment docs.

@awgreene awgreene marked this pull request as ready for review February 23, 2022 14:50
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2022
@openshift-ci openshift-ci bot requested review from jmrodri and njhale February 23, 2022 14:51
exdx
exdx reviewed Feb 23, 2022
View reviewed changes
Copy link
Contributor

@exdx exdx left a comment

Choose a reason for hiding this comment

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

/lgtm

go.mod
sigs.k8s.io/yaml v1.2.0
)

require (
Copy link
Contributor

@exdx exdx Feb 23, 2022

Choose a reason for hiding this comment

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

This looks funny -- should we have two require() blocks in one go.mod file? If I'm reading this right...

Copy link
Member

@timflannagan timflannagan Feb 23, 2022

Choose a reason for hiding this comment

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

This kind of pattern is more-or-less standard now with how Go 1.17 handles go.mod files now: https://go.dev/doc/go1.17#go-command.

Copy link
Member Author

@awgreene awgreene Feb 23, 2022

Choose a reason for hiding this comment

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

Odd, going to look into this.

/hold

Copy link
Member Author

@awgreene awgreene Feb 23, 2022

Choose a reason for hiding this comment

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

/unhold
https://stackoverflow.com/questions/69432877/why-there-are-two-require-blocks-in-go-mod-since-go-1-17?noredirect=1&lq=1

Copy link
Contributor

@exdx exdx Feb 23, 2022

Choose a reason for hiding this comment

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

Ok thanks for the info 👍

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2022
timflannagan
timflannagan reviewed Feb 23, 2022
View reviewed changes
Dockerfile
@@ -1,4 +1,4 @@
FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.16-openshift-4.8 AS builder
FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.17-openshift-4.8 AS builder
Copy link
Member

@timflannagan timflannagan Feb 23, 2022

Choose a reason for hiding this comment

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

TIL we still have this Dockerfile and it was missed when we were separating out the downstream builds. Maybe this is something we can rework in the future to avoid using non-external images? cc @joelanford

Copy link
Member

@joelanford joelanford Feb 23, 2022

Choose a reason for hiding this comment

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

Yeah, in general I think there's a bunch of cleanup of Dockerfiles and quay build triggers we can do in this repo now.

Do we need anything other than the quay.io/operator-framework/opm image at this point?

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2022
timflannagan
timflannagan reviewed Feb 23, 2022
View reviewed changes
test/e2e/ctx/provisioner_kubeconfig.go
@@ -1,3 +1,4 @@
//go:build !kind
Copy link
Member

@timflannagan timflannagan Feb 23, 2022

Choose a reason for hiding this comment

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

unrelated: This points to the need for a shared operator-framework/test-infra repository so we're not de-duplicating work across the registry and OLM repositories.

@timflannagan
Copy link
Member

timflannagan commented Feb 23, 2022

/lgtm

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2022
dinhxuanvu
dinhxuanvu reviewed Feb 23, 2022
View reviewed changes
@awgreene
Bump go to v1.17
e06edc2 
This commit updates the operator-registry's go version
to v1.17.

Signed-off-by: Alexander Greene <greene.al1991@gmail.com>
dinhxuanvu
dinhxuanvu approved these changes Feb 23, 2022
View reviewed changes
Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awgreene, dinhxuanvu

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2022
@timflannagan
Copy link
Member

timflannagan commented Feb 23, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2022
@openshift-merge-robot openshift-merge-robot merged commit c970d61 into operator-framework:master Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joelanford joelanford joelanford left review comments

@dinhxuanvu dinhxuanvu dinhxuanvu approved these changes

@gallettilance gallettilance Awaiting requested review from gallettilance

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@jmrodri jmrodri Awaiting requested review from jmrodri

@njhale njhale Awaiting requested review from njhale

+2 more reviewers

@timflannagan timflannagan timflannagan left review comments

@exdx exdx exdx left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@timflannagan timflannagan

@exdx exdx

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@awgreene @timflannagan @joelanford @dinhxuanvu @exdx @openshift-merge-robot